Bug 18936: (follow-up) fix tests, null vs. empty behavior for limiting rules

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 6c10b3d..0027f81 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -473,7 +473,7 @@ sub TooMany {
         my $max_checkouts_allowed = $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef;
         my $max_onsite_checkouts_allowed = $maxonsiteissueqty_rule ? $maxonsiteissueqty_rule->rule_value : undef;
 
-        if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) {
+        if ( $onsite_checkout and $max_onsite_checkouts_allowed ne '' ) {
             if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
                 return {
                     reason => 'TOO_MANY_ONSITE_CHECKOUTS',
@@ -2303,7 +2303,7 @@ sub _calculate_new_debar_dt {
         # If the max suspension days is < than the suspension days
         # the suspension days is limited to this maximum period.
         my $max_sd = $issuing_rule->{maxsuspensiondays};
-        if ( defined $max_sd ) {
+        if ( defined $max_sd && $max_sd ne '' ) {
             $max_sd = DateTime::Duration->new( days => $max_sd );
             $suspension_days = $max_sd
               if DateTime::Duration->compare( $max_sd, $suspension_days ) < 0;

diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl
index 67f72e6..8a22807 100755 (executable)
--- a/admin/smart-rules.pl
+++ b/admin/smart-rules.pl
@@ -253,6 +253,7 @@ elsif ($op eq 'add') {
     my $finedays     = $input->param('finedays');
     my $maxsuspensiondays = $input->param('maxsuspensiondays');
     $maxsuspensiondays = undef if $maxsuspensiondays eq q||;
+    $maxsuspensiondays = '' if $maxsuspensiondays eq q||;
     my $suspension_chargeperiod = $input->param('suspension_chargeperiod') || 1;
     my $firstremind  = $input->param('firstremind');
     my $chargeperiod = $input->param('chargeperiod');
@@ -262,11 +263,11 @@ elsif ($op eq 'add') {
     my $renewalsallowed  = $input->param('renewalsallowed');
     my $renewalperiod    = $input->param('renewalperiod');
     my $norenewalbefore  = $input->param('norenewalbefore');
-    $norenewalbefore = undef if $norenewalbefore =~ /^\s*$/;
+    $norenewalbefore = '' if $norenewalbefore =~ /^\s*$/;
     my $auto_renew = $input->param('auto_renew') eq 'yes' ? 1 : 0;
     my $no_auto_renewal_after = $input->param('no_auto_renewal_after');
-    $no_auto_renewal_after = undef if $no_auto_renewal_after =~ /^\s*$/;
-    my $no_auto_renewal_after_hard_limit = $input->param('no_auto_renewal_after_hard_limit') || undef;
+    $no_auto_renewal_after = '' if $no_auto_renewal_after =~ /^\s*$/;
+    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');
@@ -289,7 +290,7 @@ elsif ($op eq 'add') {
     my $rentaldiscount = $input->param('rentaldiscount');
     my $opacitemholds = $input->param('opacitemholds') || 0;
     my $article_requests = $input->param('article_requests') || 'no';
-    my $overduefinescap = $input->param('overduefinescap') || undef;
+    my $overduefinescap = $input->param('overduefinescap') || '';
     my $cap_fine_to_replacement_price = $input->param('cap_fine_to_replacement_price') eq 'on';
     my $note = $input->param('note');
     $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $maxonsiteissueqty, $cap_fine_to_replacement_price";

diff --git a/installer/data/mysql/atomicupdate/bug_18936.perl b/installer/data/mysql/atomicupdate/bug_18936.perl
index b420fce..935f718 100644 (file)
--- a/installer/data/mysql/atomicupdate/bug_18936.perl
+++ b/installer/data/mysql/atomicupdate/bug_18936.perl
@@ -27,13 +27,15 @@ if( CheckVersion( $DBversion ) ) {
         onshelfholds
         opacitemholds
         article_requests
+        maxissueqty
+        maxonsiteissueqty
     );
 
     if ( column_exists( 'issuingrules', 'categorycode' ) ) {
         foreach my $column ( @columns ) {
             $dbh->do("
                 INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
-                SELECT categorycode, branchcode, itemtype, \'$column\', $column
+                SELECT categorycode, branchcode, itemtype, \'$column\', COALESCE( $column, '' )
                 FROM issuingrules
             ");
         }

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
index 7ad46a2..8bb1381 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
@@ -185,14 +185,14 @@
                                         [% ELSE %]<span>&nbsp;</span>[% END %]
                                     </td>
                                     <td>
-                                        [% IF maxissueqty  %]
+                                        [% IF maxissueqty.defined && maxissueqty != '' %]
                                             [% maxissueqty %]
                                         [% ELSE %]
                                             <span>Unlimited</span>
                                         [% END %]
                                     </td>
                                     <td>
-                                        [% IF maxonsiteissueqty  %]
+                                        [% IF maxonsiteissueqty.defined && maxonsiteissueqty != ''  %]
                                             [% maxonsiteissueqty %]
                                         [% ELSE %]
                                             <span>Unlimited</span>

diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
index cebe7dc..2bff2e4 100755 (executable)
--- a/t/db_dependent/Circulation.t
+++ b/t/db_dependent/Circulation.t
@@ -914,7 +914,17 @@ subtest "CanBookBeRenewed tests" => sub {
             }
         });
 
-        $dbh->do('UPDATE issuingrules SET norenewalbefore = 10, no_auto_renewal_after = 11');
+        Koha::CirculationRules->set_rules(
+            {
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
+                rules        => {
+                    norenewalbefore       => 10,
+                    no_auto_renewal_after => 11,
+                }
+            }
+        );
 
         my $ten_days_before = dt_from_string->add( days => -10 );
         my $ten_days_ahead = dt_from_string->add( days => 10 );
@@ -2892,26 +2902,23 @@ subtest 'CanBookBeIssued | is_overdue' => sub {
     plan tests => 3;
 
     # Set a simple circ policy
-    $dbh->do('DELETE FROM issuingrules');
-    $dbh->do(
-    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed,
-                                    issuelength, lengthunit,
-                                    renewalsallowed, renewalperiod,
-                                    norenewalbefore, auto_renew,
-                                    fine, chargeperiod)
-          VALUES (?, ?, ?, ?,
-                  ?, ?,
-                  ?, ?,
-                  ?, ?,
-                  ?, ?
-                 )
-        },
-        {},
-        '*',   '*', '*', 25,
-        14,  'days',
-        1,     7,
-        undef, 0,
-        .10,   1
+    Koha::CirculationRules->set_rules(
+        {
+            categorycode => undef,
+            branchcode   => undef,
+            itemtype     => undef,
+            rules        => {
+                reservesallowed => 25,
+                issuelength     => 14,
+                lengthunit      => 'days',
+                renewalsallowed => 1,
+                renewalperiod   => 7,
+                norenewalbefore => undef,
+                auto_renew      => 0,
+                fine            => .10,
+                chargeperiod    => 1,
+            }
+        }
     );
 
     my $five_days_go = output_pref({ dt => dt_from_string->add( days => 5 ), dateonly => 1});

diff --git a/t/db_dependent/Circulation/TooMany.t b/t/db_dependent/Circulation/TooMany.t
index 7e45844..8a7cf3a 100644 (file)
--- a/t/db_dependent/Circulation/TooMany.t
+++ b/t/db_dependent/Circulation/TooMany.t
@@ -164,7 +164,7 @@ subtest '1 Issuingrule exist with onsiteissueqty=unlimited' => sub {
         {
             branchcode   => $branch->{branchcode},
             categorycode => $category->{categorycode},
-            itemtype     => '*',
+            itemtype     => undef,
             rules        => {
                 maxissueqty       => 1,
                 maxonsiteissueqty => undef,