Attachment 99171 Details for Bug 24669 – Bug 24669: Improve numeric input handling in smart-rules.pl

[patch] Bug 24669: Improve numeric input handling in smart-rules.pl

Bug-24669-Improve-numeric-input-handling-in-smart-.patch (text/plain), 5.78 KB, created by Jonathan Druart on 2020-02-18 10:50:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2020-02-18 10:50:00 UTC

Size: 5.78 KB

patch

obsolete

>From 64c339ba1bb766ac61010f4269be23c78ae0fb97 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@helsinki.fi>
>Date: Mon, 17 Feb 2020 09:48:22 +0000
>Subject: [PATCH] Bug 24669: Improve numeric input handling in smart-rules.pl
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>This removes code duplication by introducing a new subroutine
>strip_non_numeric() that removes whitespace and makes the rule value
>'' if it is a string value instead of a digit. The call to
>strip_non_numeric() is now added also to all the rules it is needed
>in.
>
>To test:
>1. Write some string like "test" to rules:
>  - Current checkouts allowed
>  - Current on-site checkouts allowed
>  - Holds allowed (total)
>  - Holds allowed (daily)
>  - Holds per record (count)
>  - Holds per record (count) â can be found in default policy
>2. Save rules
>3. Apply patch
>4. Edit the rule line where you just typed all those strings and save
>5. Notice how the values are now empty strings (some say "unlimited" but
>   if you go check from DB it should show empty value there)
>
>Sponsored-by: The National Library of Finland
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>---
> admin/smart-rules.pl | 41 +++++++++++++++++------------------------
> 1 file changed, 17 insertions(+), 24 deletions(-)
>
>diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl
>index 75ed5164ca..dc8033b7f5 100755
>--- a/admin/smart-rules.pl
>+++ b/admin/smart-rules.pl
>@@ -261,8 +261,8 @@ elsif ($op eq 'add') {
>     my $firstremind  = $input->param('firstremind');
>     my $chargeperiod = $input->param('chargeperiod');
>     my $chargeperiod_charge_at = $input->param('chargeperiod_charge_at');
>-    my $maxissueqty  = $input->param('maxissueqty');
>-    my $maxonsiteissueqty  = $input->param('maxonsiteissueqty');
>+    my $maxissueqty  = strip_non_numeric($input->param('maxissueqty'));
>+    my $maxonsiteissueqty  = strip_non_numeric($input->param('maxonsiteissueqty'));
>     my $renewalsallowed  = $input->param('renewalsallowed');
>     my $renewalperiod    = $input->param('renewalperiod');
>     my $norenewalbefore  = $input->param('norenewalbefore');
>@@ -273,16 +273,10 @@ elsif ($op eq 'add') {
>     my $no_auto_renewal_after_hard_limit = $input->param('no_auto_renewal_after_hard_limit') || '';
>     $no_auto_renewal_after_hard_limit = eval { dt_from_string( $input->param('no_auto_renewal_after_hard_limit') ) } if ( $no_auto_renewal_after_hard_limit );
>     $no_auto_renewal_after_hard_limit = output_pref( { dt => $no_auto_renewal_after_hard_limit, dateonly => 1, dateformat => 'iso' } ) if ( $no_auto_renewal_after_hard_limit );
>-    my $reservesallowed  = $input->param('reservesallowed');
>-    my $holds_per_record = $input->param('holds_per_record');
>-    my $holds_per_day    = $input->param('holds_per_day');
>-    $holds_per_day =~ s/\s//g;
>-    $holds_per_day = '' if $holds_per_day !~ /^\d+/;
>+    my $reservesallowed  = strip_non_numeric($input->param('reservesallowed'));
>+    my $holds_per_record = strip_non_numeric($input->param('holds_per_record'));
>+    my $holds_per_day    = strip_non_numeric($input->param('holds_per_day'));
>     my $onshelfholds     = $input->param('onshelfholds') || 0;
>-    $maxissueqty =~ s/\s//g;
>-    $maxissueqty = '' if $maxissueqty !~ /^\d+/;
>-    $maxonsiteissueqty =~ s/\s//g;
>-    $maxonsiteissueqty = '' if $maxonsiteissueqty !~ /^\d+/;
>     my $issuelength  = $input->param('issuelength');
>     $issuelength = $issuelength eq q{} ? undef : $issuelength;
>     my $lengthunit  = $input->param('lengthunit');
>@@ -342,20 +336,15 @@ elsif ($op eq 'add') {
> }
> elsif ($op eq "set-branch-defaults") {
>     my $categorycode  = $input->param('categorycode');
>-    my $patron_maxissueqty   = $input->param('patron_maxissueqty');
>+    my $patron_maxissueqty   = strip_non_numeric($input->param('patron_maxissueqty'));
>     my $patron_maxonsiteissueqty = $input->param('patron_maxonsiteissueqty');
>+    $patron_maxonsiteissueqty = strip_non_numeric($patron_maxonsiteissueqty);
>     my $holdallowed   = $input->param('holdallowed');
>     my $hold_fulfillment_policy = $input->param('hold_fulfillment_policy');
>     my $returnbranch  = $input->param('returnbranch');
>-    my $max_holds = $input->param('max_holds');
>-    $patron_maxissueqty =~ s/\s//g;
>-    $patron_maxissueqty = '' if $patron_maxissueqty !~ /^\d+/;
>-    $patron_maxonsiteissueqty =~ s/\s//g;
>-    $patron_maxonsiteissueqty = '' if $patron_maxonsiteissueqty !~ /^\d+/;
>+    my $max_holds = strip_non_numeric($input->param('max_holds'));
>     $holdallowed =~ s/\s//g;
>     $holdallowed = undef if $holdallowed !~ /^\d+/;
>-    $max_holds =~ s/\s//g;
>-    $max_holds = '' if $max_holds !~ /^\d+/;
> 
>     if ($branch eq "*") {
>         Koha::CirculationRules->set_rules(
>@@ -413,13 +402,10 @@ elsif ($op eq "set-branch-defaults") {
> }
> elsif ($op eq "add-branch-cat") {
>     my $categorycode  = $input->param('categorycode');
>-    my $patron_maxissueqty   = $input->param('patron_maxissueqty');
>+    my $patron_maxissueqty = strip_non_numeric($input->param('patron_maxissueqty'));
>     my $patron_maxonsiteissueqty = $input->param('patron_maxonsiteissueqty');
>+    $patron_maxonsiteissueqty = strip_non_numeric($patron_maxonsiteissueqty);
>     my $max_holds = $input->param('max_holds');
>-    $patron_maxissueqty =~ s/\s//g;
>-    $patron_maxissueqty = '' if $patron_maxissueqty !~ /^\d+/;
>-    $patron_maxonsiteissueqty =~ s/\s//g;
>-    $patron_maxonsiteissueqty = '' if $patron_maxonsiteissueqty !~ /^\d+/;
>     $max_holds =~ s/\s//g;
>     $max_holds = undef if $max_holds !~ /^\d+/;
> 
>@@ -620,3 +606,10 @@ sub by_itemtype {
>         return lc $a->{'translated_description'} cmp lc $b->{'translated_description'};
>     }
> }
>+
>+sub strip_non_numeric {
>+    my $string = shift;
>+    $string =~ s/\s//g;
>+    $string = '' if $string !~ /^\d+/;
>+    return $string;
>+}
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 24669: 99079 | 99080 | 99171 | 99172 | 99216 | 99217 | 99251