@@ -, +, @@ that weren't saved to the database - issuelength - hardduedate - unseenrenewalsallowed - rentaldiscount - decreaseloanholds Patron category: Teacher Itemtype: All Hard due date: (Today) Lona period: 10 Patron category: Teacher Itemtype: Books Hard due date: (leave blank) Loan period: 10 --- C4/Circulation.pm | 4 +- admin/smart-rules.pl | 31 +++++++------- .../data/mysql/atomicupdate/bug_29012.pl | 41 +++++++++++++++++++ installer/onboarding.pl | 8 ++-- .../prog/en/modules/admin/smart-rules.tt | 8 +++- 5 files changed, 69 insertions(+), 23 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_29012.pl --- a/C4/Circulation.pm +++ a/C4/Circulation.pm @@ -2885,7 +2885,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'); @@ -3099,7 +3099,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 --- a/admin/smart-rules.pl +++ a/admin/smart-rules.pl @@ -255,7 +255,7 @@ elsif ($op eq 'add') { my $itemtype = $input->param('itemtype'); # item type my $fine = $input->param('fine'); my $finedays = $input->param('finedays'); - my $maxsuspensiondays = $input->param('maxsuspensiondays') || ''; + my $maxsuspensiondays = $input->param('maxsuspensiondays') || q{}; my $suspension_chargeperiod = $input->param('suspension_chargeperiod') || 1; my $firstremind = $input->param('firstremind'); my $chargeperiod = $input->param('chargeperiod'); @@ -263,35 +263,34 @@ 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 = defined $input->param('unseen_renewals_allowed') ? strip_non_numeric( scalar $input->param('unseen_renewals_allowed') ) : q{}; my $renewalperiod = $input->param('renewalperiod'); my $norenewalbefore = $input->param('norenewalbefore'); - $norenewalbefore = '' if $norenewalbefore =~ /^\s*$/; + $norenewalbefore = q{} 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 = '' 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 = q{} if $no_auto_renewal_after =~ /^\s*$/; + my $no_auto_renewal_after_hard_limit = $input->param('no_auto_renewal_after_hard_limit') || q{}; $no_auto_renewal_after_hard_limit = eval { dt_from_string( scalar $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 = strip_non_numeric( scalar $input->param('reservesallowed') ); 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') || q{}; $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 $overduefinescap = $input->param('overduefinescap') || q{}; + my $cap_fine_to_replacement_price = ($input->param('cap_fine_to_replacement_price') || q{}) eq 'on'; my $note = $input->param('note'); - my $decreaseloanholds = $input->param('decreaseloanholds') || undef; + my $decreaseloanholds = $input->param('decreaseloanholds') || q{}; 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'); @@ -481,7 +480,7 @@ elsif ( $op eq "add-open-article-requests-limit" ) { Koha::Exception->throw("No value passed for article request limit") if not defined $open_article_requests_limit # There is a JS check for that - || $open_article_requests_limit eq ''; + || $open_article_requests_limit eq q{}; if ( $branch eq "*" ) { if ( $categorycode eq "*" ) { @@ -555,7 +554,7 @@ elsif ( $op eq "set-article-request-fee" ) { Koha::Exception->throw("No value passed for article request fee") if not defined $fee # There is a JS check for that - || $fee eq ''; + || $fee eq q{}; Koha::CirculationRules->set_rules( { categorycode => ( $category eq '*' ) ? undef : $category, @@ -713,7 +712,7 @@ my $definedbranch = $all_rules->count ? 1 : 0; my $rules = {}; while ( my $r = $all_rules->next ) { $r = $r->unblessed; - $rules->{ $r->{categorycode} // '' }->{ $r->{itemtype} // '' }->{ $r->{rule_name} } = $r->{rule_value}; + $rules->{ $r->{categorycode} // q{} }->{ $r->{itemtype} // q{} }->{ $r->{rule_name} } = $r->{rule_value}; } $template->param(show_branch_cat_rule_form => 1); @@ -763,6 +762,6 @@ sub by_itemtype { sub strip_non_numeric { my $string = shift; $string =~ s/\s//g; - $string = '' if $string !~ /^\d+/; + $string = q{} if $string !~ /^\d+/; return $string; } --- a/installer/data/mysql/atomicupdate/bug_29012.pl +++ a/installer/data/mysql/atomicupdate/bug_29012.pl @@ -0,0 +1,41 @@ +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 => '', + unseen_renewals_allowed => '', + 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 EXISTS ( + SELECT * FROM circulation_rules cr2 + WHERE + cr2.rule_name="suspension_chargeperiod" + 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) ) ) + ) + AND 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"; + }, +} --- a/installer/onboarding.pl +++ a/installer/onboarding.pl @@ -230,7 +230,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'); @@ -240,7 +240,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, @@ -274,8 +273,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, --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -308,7 +308,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 %] --