From 387900a65c7cafcf28ad0da415e4136ffa7e0443 Mon Sep 17 00:00:00 2001 From: Kevin Carnes 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 Signed-off-by: Caroline Cyr La Rose Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi (cherry picked from commit cf397ac3bcbc93a508954c836d1cb90a84fb2ac6) Signed-off-by: Lucas Gass --- C4/Circulation.pm | 4 +-- admin/smart-rules.pl | 11 +++---- .../data/mysql/atomicupdate/bug_29012.pl | 33 +++++++++++++++++++ installer/onboarding.pl | 8 ++--- .../prog/en/modules/admin/smart-rules.tt | 8 ++++- 5 files changed, 51 insertions(+), 13 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_29012.pl diff --git a/C4/Circulation.pm b/C4/Circulation.pm index f86c25dc11..a74675e847 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2900,7 +2900,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'); @@ -3143,7 +3143,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 b7f2ae643e..4e7a997270 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -264,7 +264,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*$/; @@ -278,21 +278,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 $recalls_allowed = $input->param('recalls_allowed'); my $recalls_per_record = $input->param('recalls_per_record'); my $on_shelf_recalls = $input->param('on_shelf_recalls'); 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/installer/onboarding.pl b/installer/onboarding.pl index 7c15b74c5b..95e2b7b28e 100755 --- a/installer/onboarding.pl +++ b/installer/onboarding.pl @@ -242,7 +242,7 @@ if ( $step == 5 ) { my $categorycode = $input->param('categorycode'); my $itemtype = $input->param('itemtype'); my $maxissueqty = $input->param('maxissueqty'); - my $issuelength = $input->param('issuelength'); + my $issuelength = $input->param('issuelength') || 0; my $lengthunit = $input->param('lengthunit'); my $renewalsallowed = $input->param('renewalsallowed'); my $renewalperiod = $input->param('renewalperiod'); @@ -252,7 +252,6 @@ if ( $step == 5 ) { my $onshelfholds = $input->param('onshelfholds') || 0; $maxissueqty =~ s/\s//g; $maxissueqty = undef if $maxissueqty !~ /^\d+/; - $issuelength = $issuelength eq q{} ? undef : $issuelength; my $params = { branchcode => $branchcode, @@ -286,8 +285,9 @@ if ( $step == 5 ) { overduefinescap => "", rentaldiscount => 0, reservesallowed => $reservesallowed, - suspension_chargeperiod => undef, - decreaseloanholds => undef, + suspension_chargeperiod => 1, + decreaseloanholds => "", + unseen_renewals_allowed => "", recalls_allowed => undef, recalls_per_record => undef, on_shelf_recalls => undef, 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 1e8bc8907e..0c4ec4c750 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 @@ -303,7 +303,13 @@ [% suspension_chargeperiod | html %] [% renewalsallowed | html %] [% IF Koha.Preference('UnseenRenewals') %] - [% unseenrenewalsallowed | html %] + + [% IF unseenrenewalsallowed.defined && unseenrenewalsallowed != '' %] + [% unseenrenewalsallowed | html %] + [% ELSE %] + Unlimited + [% END %] + [% END %] [% renewalperiod | html %] [% norenewalbefore | html %] -- 2.39.5