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 <caroline.cyr-la-rose@inlibro.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit cf397ac3bc)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 387900a65c)
Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
This commit is contained in:
Kevin Carnes 2021-12-03 09:49:59 +00:00 committed by Arthur Suzuki
parent cab603e503
commit c4829c0eac
5 changed files with 48 additions and 11 deletions

View File

@ -2782,7 +2782,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');
@ -3008,7 +3008,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

View File

@ -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,

View File

@ -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";
},
}

View File

@ -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,

View File

@ -281,7 +281,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>