From 114c1dfb2c0d8a14ea12e800ab57f1f942d0aa2b Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 2 Feb 2018 08:11:55 +0100 Subject: [PATCH] Bug 15494: (QA follow-up) Additional polishing [1] Fix two typos in Circulation.t. Although the test does not fail, line 2127 contains two typos. Changing INVISILE to INVISIBLE :) And type should be itype. [2] Remove $yaml as leftover from older code. [3] Add a next when the split on /:/ does not give two results. This will prevent uninit warnings (although still disabled now in Circulation). [4] For the same reason we should switch the lines for NULL and empty string. The undefs you insert should trigger a warn. [5] The line for empty string should not insert undef, but empty string. For the same reason adding the condition defined($_) ... And proving it by adding two tests for the opposite values of callnumber and itemnotes. [6] Adding a strip spaces around the fieldname. User friendly.. Signed-off-by: Marcel de Rooy Signed-off-by: Katrin Fischer Signed-off-by: Nick Clemens --- C4/Circulation.pm | 9 +++++---- t/db_dependent/Circulation.t | 14 ++++++++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index a929c6ca25..56ef56920f 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -4130,15 +4130,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; @@ -4148,7 +4149,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; diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index ab3435fa38..83732f37c0 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -2461,7 +2461,7 @@ subtest 'CanBookBeIssued | is_overdue' => sub { }; subtest 'ItemsDeniedRenewal preference' => sub { - plan tests => 16; + plan tests => 18; t::lib::Mocks::mock_preference( 'ItemsDeniedRenewal', '' ); @@ -2538,7 +2538,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 ) = @@ -2567,7 +2567,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 ) = -- 2.39.5