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 <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This commit is contained in:
Marcel de Rooy 2018-02-02 08:11:55 +01:00 committed by Nick Clemens
parent b170f77d10
commit 114c1dfb2c
2 changed files with 17 additions and 6 deletions

View file

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

View file

@ -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 ) =