@@ -, +, @@ Although the test does not fail, line 2127 contains two typos. Changing INVISILE to INVISIBLE :) And type should be itype. prevent uninit warnings (although still disabled now in Circulation). string. The undefs you insert should trigger a warn. For the same reason adding the condition defined($_) ... And proving it by adding two tests for the opposite values of callnumber and itemnotes. --- C4/Circulation.pm | 9 +++++---- t/db_dependent/Circulation.t | 14 ++++++++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) --- a/C4/Circulation.pm +++ a/C4/Circulation.pm @@ -4089,15 +4089,16 @@ sub _item_denied_renewal { my $item = $params->{item}; return unless $item; - my $yaml = C4::Context->preference('ItemsDeniedRenewal'); - my @lines = split /\n/, $yaml; + my @lines = split /\n/, C4::Context->preference('ItemsDeniedRenewal')//''; my $denyingrules; foreach my $line (@lines){ my ($field,$array) = split /:/, $line; + next if !$array; + $field =~ s/^\s*|\s*$//g; $array =~ s/[ [\]\r]//g; my @array = split /,/, $array; + @array = map { $_ eq '""' || $_ eq "''" ? '' : $_ } @array; @array = map { $_ eq 'NULL' ? undef : $_ } @array; - @array = map { $_ eq '""' || $_ eq "''" ? undef : $_ } @array; $denyingrules->{$field} = \@array; } return unless $denyingrules; @@ -4107,7 +4108,7 @@ sub _item_denied_renewal { if ( any { !defined $_ } @{$denyingrules->{$field}} ){ return 1; } - } elsif (any { $val eq $_ } @{$denyingrules->{$field}}) { + } elsif (any { defined($_) && $val eq $_ } @{$denyingrules->{$field}}) { # If the results matches the values in the syspref # We return true if match found return 1; --- a/t/db_dependent/Circulation.t +++ a/t/db_dependent/Circulation.t @@ -2047,7 +2047,7 @@ subtest 'CanBookBeIssued | is_overdue' => sub { }; subtest 'ItemsDeniedRenewal preference' => sub { - plan tests => 16; + plan tests => 18; t::lib::Mocks::mock_preference( 'ItemsDeniedRenewal', '' ); @@ -2124,7 +2124,7 @@ subtest 'ItemsDeniedRenewal preference' => sub { is( $idr_mayrenew, 1, 'Renewal allowed when 1 rules not matched (withdrawn)' ); is( $idr_error, undef, 'Renewal allowed when 1 rules not matched (withdrawn)' ); - $idr_rules="withdrawn: [1]\ntype: [HIDE,INVISILE]"; + $idr_rules="withdrawn: [1]\nitype: [HIDE,INVISIBLE]"; t::lib::Mocks::mock_preference( 'ItemsDeniedRenewal', $idr_rules ); ( $idr_mayrenew, $idr_error ) = @@ -2153,7 +2153,17 @@ subtest 'ItemsDeniedRenewal preference' => sub { ( $idr_mayrenew, $idr_error ) = CanBookBeRenewed( $idr_borrower->borrowernumber, $deny_issue->itemnumber ); is( $idr_mayrenew, 0, 'Renewal blocked for undef when NULL in pref' ); + $idr_rules="itemcallnumber: ['']"; + t::lib::Mocks::mock_preference( 'ItemsDeniedRenewal', $idr_rules ); + ( $idr_mayrenew, $idr_error ) = + CanBookBeRenewed( $idr_borrower->borrowernumber, $deny_issue->itemnumber ); + is( $idr_mayrenew, 1, 'Renewal not blocked for undef when "" in pref' ); + $idr_rules="itemnotes: [NULL]"; + t::lib::Mocks::mock_preference( 'ItemsDeniedRenewal', $idr_rules ); + ( $idr_mayrenew, $idr_error ) = + CanBookBeRenewed( $idr_borrower->borrowernumber, $deny_issue->itemnumber ); + is( $idr_mayrenew, 1, 'Renewal not blocked for "" when NULL in pref' ); $idr_rules="itemnotes: ['']"; t::lib::Mocks::mock_preference( 'ItemsDeniedRenewal', $idr_rules ); ( $idr_mayrenew, $idr_error ) = --