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>
This commit is contained in:
Kevin Carnes
parent
a39825d1a1
commit
387900a65c
5 changed files with 51 additions and 13 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
|
|
|||
33
installer/data/mysql/atomicupdate/bug_29012.pl
Executable file
33
installer/data/mysql/atomicupdate/bug_29012.pl
Executable 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";
|
||||
},
|
||||
}
|
||||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -303,7 +303,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>
|
||||
|
|
|
|||
Loading…
Reference in a new issue