Attachment 131053 Details for Bug 29012 – Bug 29012: Add default values for blank circulation rules that weren't saved to the database

[patch] Bug 29012: Add default values for blank circulation rules that weren't saved to the database

Bug-29012-Add-default-values-for-blank-circulation.patch (text/plain), 8.05 KB, created by Kevin Carnes on 2022-02-24 07:34:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kevin Carnes

Created: 2022-02-24 07:34:48 UTC

Size: 8.05 KB

patch

obsolete

>From 1325fafcb30c7a0cb057eebade33a018961f4ec1 Mon Sep 17 00:00:00 2001
>From: Kevin Carnes <kevin.carnes@ub.lu.se>
>Date: Fri, 3 Dec 2021 09:49:59 +0000
>Subject: [PATCH] Bug 29012: Add default values for blank circulation rules
> that weren't saved to the database
>
>There are 5 fields that are not set if no value is provided when saving/editing a rule in Administration->Circulation and fines rules
>- issuelength
>- hardduedate
>- unseenrenewalsallowed
>- rentaldiscount
>- decreaseloanholds
>
>This is problematic because it gives the impression these rules are set as blank, but in reality they don't exist and the rule will fal back to the higher level
>
>To test:
>1 - Set a rule for
>    Patron category: Teacher
>    Itemtype: All
>    Hard due date: (Today)
>    Lona period: 10
>2 - Set a rule for
>    Patron category: Teacher
>    Itemtype: Books
>    Hard due date: (leave blank)
>    Loan period: 10
>3 - Expected behaviour is Book item will checkout to teacher for 10 days, all other types will be due yesterday at 25:59:00
>4 - Checkout an non-book item type to teacher
>5 - Hard due date applies
>6 - Checkout a 'book' item type to teacher
>7 - Hard due date applies - FAIL
>---
> C4/Circulation.pm                             |  4 +--
> admin/smart-rules.pl                          | 11 +++----
> .../data/mysql/atomicupdate/bug_29012.pl      | 33 +++++++++++++++++++
> .../prog/en/modules/admin/smart-rules.tt      |  8 ++++-
> 4 files changed, 47 insertions(+), 9 deletions(-)
> create mode 100755 installer/data/mysql/atomicupdate/bug_29012.pl
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 17cab0c74b..74fdab7e03 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -2778,7 +2778,7 @@ sub CanBookBeRenewed {
> 
>         return ( 0, "too_unseen" )
>           if C4::Context->preference('UnseenRenewals') &&
>-            $issuing_rule->{unseen_renewals_allowed} &&
>+            looks_like_number($issuing_rule->{unseen_renewals_allowed}) &&
>             $issuing_rule->{unseen_renewals_allowed} <= $issue->unseen_renewals;
> 
>         my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing');
>@@ -2985,7 +2985,7 @@ sub AddRenewal {
>                     rule_name    => 'unseen_renewals_allowed'
>                 }
>             );
>-            if (!$seen && $rule && $rule->rule_value) {
>+            if (!$seen && $rule && looks_like_number($rule->rule_value)) {
>                 $unseen_renewals++;
>             } else {
>                 # If the renewal is seen, unseen should revert to 0
>diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl
>index 8f429ec6ae..fb5ab858ee 100755
>--- a/admin/smart-rules.pl
>+++ b/admin/smart-rules.pl
>@@ -258,7 +258,7 @@ elsif ($op eq 'add') {
>     my $maxissueqty = strip_non_numeric( scalar $input->param('maxissueqty') );
>     my $maxonsiteissueqty = strip_non_numeric( scalar $input->param('maxonsiteissueqty') );
>     my $renewalsallowed  = $input->param('renewalsallowed');
>-    my $unseen_renewals_allowed  = $input->param('unseen_renewals_allowed');
>+    my $unseen_renewals_allowed  = strip_non_numeric( scalar $input->param('unseen_renewals_allowed') ) // '';
>     my $renewalperiod    = $input->param('renewalperiod');
>     my $norenewalbefore  = $input->param('norenewalbefore');
>     $norenewalbefore = '' if $norenewalbefore =~ /^\s*$/;
>@@ -272,21 +272,20 @@ elsif ($op eq 'add') {
>     my $holds_per_record = strip_non_numeric( scalar $input->param('holds_per_record') );
>     my $holds_per_day    = strip_non_numeric( scalar $input->param('holds_per_day') );
>     my $onshelfholds     = $input->param('onshelfholds') || 0;
>-    my $issuelength  = $input->param('issuelength');
>-    $issuelength = $issuelength eq q{} ? undef : $issuelength;
>+    my $issuelength  = $input->param('issuelength') || 0;
>     my $daysmode = $input->param('daysmode');
>     my $lengthunit  = $input->param('lengthunit');
>-    my $hardduedate = $input->param('hardduedate') || undef;
>+    my $hardduedate = $input->param('hardduedate') || '';
>     $hardduedate = eval { dt_from_string( scalar $hardduedate ) } if ( $hardduedate );
>     $hardduedate = output_pref( { dt => $hardduedate, dateonly => 1, dateformat => 'iso' } ) if ( $hardduedate );
>     my $hardduedatecompare = $input->param('hardduedatecompare');
>-    my $rentaldiscount = $input->param('rentaldiscount');
>+    my $rentaldiscount = $input->param('rentaldiscount') || 0;
>     my $opacitemholds = $input->param('opacitemholds') || 0;
>     my $article_requests = $input->param('article_requests') || 'no';
>     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');
>-    my $decreaseloanholds = $input->param('decreaseloanholds') || undef;
>+    my $decreaseloanholds = $input->param('decreaseloanholds') || '';
> 
>     my $rules = {
>         maxissueqty                   => $maxissueqty,
>diff --git a/installer/data/mysql/atomicupdate/bug_29012.pl b/installer/data/mysql/atomicupdate/bug_29012.pl
>new file mode 100755
>index 0000000000..26aac4b686
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug_29012.pl
>@@ -0,0 +1,33 @@
>+use Modern::Perl;
>+
>+return {
>+    bug_number => "29012",
>+    description => "Some rules are not saved when left blank while editing a 'rule' line in smart-rules.pl",
>+    up => sub {
>+        my ($args) = @_;
>+        my ($dbh, $out) = @$args{qw(dbh out)};
>+        my %default_rule_values = (
>+            issuelength             => 0,
>+            hardduedate             => '',
>+            unseenrenewalsallowed   => '',
>+            rentaldiscount          => 0,
>+            decreaseloanholds       => '',
>+        );
>+        while (my ($rule_name, $rule_value) = each (%default_rule_values)) {
>+            $dbh->do(q{
>+                INSERT IGNORE INTO circulation_rules (branchcode, categorycode, itemtype, rule_name, rule_value)
>+                    SELECT branchcode, categorycode, itemtype, ?, ? FROM circulation_rules cr
>+                        WHERE NOT EXISTS (
>+                            SELECT * FROM circulation_rules cr2
>+                                WHERE
>+                                    cr2.rule_name=?
>+                                    AND ( (cr2.branchcode=cr.branchcode) OR ( ISNULL(cr2.branchcode) AND ISNULL(cr.branchcode) ) )
>+                                    AND ( (cr2.categorycode=cr.categorycode) OR ( ISNULL(cr2.categorycode) AND ISNULL(cr.categorycode) ) )
>+                                    AND ( (cr2.itemtype=cr.itemtype) OR ( ISNULL(cr2.itemtype) AND ISNULL(cr.itemtype) ) )
>+                        )
>+                        GROUP BY branchcode, categorycode, itemtype
>+            }, undef, $rule_name, $rule_value, $rule_name);
>+        }
>+        say $out "Add default values for blank circulation rules that weren't saved to the database";
>+    },
>+}
>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 9b33d07807..208c7dbd5e 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>@@ -280,7 +280,13 @@
>                                     <td>[% suspension_chargeperiod | html %]</td>
>                                     <td>[% renewalsallowed | html %]</td>
>                                     [% IF Koha.Preference('UnseenRenewals') %]
>-                                        <td>[% unseenrenewalsallowed | html %]</td>
>+                                        <td>
>+                                            [% IF unseenrenewalsallowed.defined && unseenrenewalsallowed != '' %]
>+                                                [% unseenrenewalsallowed | html %]
>+                                            [% ELSE %]
>+                                                <span>Unlimited</span>
>+                                            [% END %]
>+                                        </td>
>                                     [% END %]
>                                     <td>[% renewalperiod | html %]</td>
>                                     <td>[% norenewalbefore | html %]</td>
>-- 
>2.17.1

Actions: View | Diff | Splinter Review

Attachments on bug 29012: 128205 | 128206 | 131043 | 131053 | 131054 | 135194 | 135998 | 138140 | 138309 | 138310 | 141869 | 141876 | 141877 | 141878 | 141879 | 141951 | 141952 | 142248 | 142249