From 4f03b5318ea9c545ab76ac37bb1308dae3c85dda Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 21 Sep 2018 20:16:24 +0000 Subject: [PATCH] Bug 15494: (follow-up) Move yaml syspref code to its own sub in SySpref object To test: 1 - prove -v t/db_dependent/Koha/Config/SysPrefs.t --- C4/Circulation.pm | 14 +------ Koha/Config/SysPref.pm | 27 ++++++++++++++ Koha/Util/SystemPreferences.pm | 69 +++++++++++++++++++++++++++++++++++ t/db_dependent/Circulation.t | 19 +++++----- t/db_dependent/Koha/Config/SysPrefs.t | 68 ++++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+), 21 deletions(-) create mode 100644 Koha/Util/SystemPreferences.pm create mode 100644 t/db_dependent/Koha/Config/SysPrefs.t diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 0ec7941..5f34516 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -57,6 +57,7 @@ use Koha::RefundLostItemFeeRule; use Koha::RefundLostItemFeeRules; use Koha::Account::Lines; use Koha::Account::Offsets; +use Koha::Config::SysPrefs; use Carp; use List::MoreUtils qw( uniq any ); use Scalar::Util qw( looks_like_number ); @@ -4108,18 +4109,7 @@ sub _item_denied_renewal { my $item = $params->{item}; return unless $item; - 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; - $denyingrules->{$field} = \@array; - } + my $denyingrules = Koha::Config::SysPrefs->find('ItemsDeniedRenewal')->get_yaml_pref_hash(); return unless $denyingrules; foreach my $field (keys %$denyingrules) { my $val = $item->{$field}; diff --git a/Koha/Config/SysPref.pm b/Koha/Config/SysPref.pm index 1e71b8c..a7cc766 100644 --- a/Koha/Config/SysPref.pm +++ b/Koha/Config/SysPref.pm @@ -37,6 +37,33 @@ Koha::Config::SysPref - Koha System Preference Object class =cut +=head3 get_yaml_pref_hash + +Turn a pref defined via YAML as a hash + +=cut + +sub get_yaml_pref_hash { + my ( $self ) = @_; + return if !defined( $self ); + + my @lines = split /\n/, $self->value//''; + my $pref_as_hash; + 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; + $pref_as_hash->{$field} = \@array; + } + + return $pref_as_hash; +} + + =head3 store =cut diff --git a/Koha/Util/SystemPreferences.pm b/Koha/Util/SystemPreferences.pm new file mode 100644 index 0000000..a57c98b --- /dev/null +++ b/Koha/Util/SystemPreferences.pm @@ -0,0 +1,69 @@ +package Koha::Util::SystemPreferences; + +# Copyright 2018 Koha Development Team +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 3 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use Modern::Perl; + +use parent qw( Exporter ); + +our @EXPORT = qw( + get_yaml_pref_hash +); + +=head1 NAME + +Koha::Util::SystemPreferences - utility class with System Preference routines + +=head1 METHODS + +=head2 get_yaml_pref_hash + +Turn a pref defined via YAML as a hash + +=cut + +sub get_yaml_pref_hash { + my ( $pref ) = @_; + return if !defined( $pref ); + + my @lines = split /\n/, C4::Context->preference($pref)//''; + my $pref_as_hash; + 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; + $pref_as_hash->{$field} = \@array; + } + + return $pref_as_hash; +} + +1; +__END__ + +=head1 AUTHOR + +Koha Development Team + +Nick Clemens + +=cut diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index a491529..f493e4e 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -226,7 +226,7 @@ C4::Context->dbh->do("DELETE FROM borrowers WHERE cardnumber = '99999999999'"); C4::Context->dbh->do("DELETE FROM accountlines"); { # CanBookBeRenewed tests - t::lib::Mocks::mock_preference( 'ItemsDeniedRenewal', '' ); #Ensure pref doesn't affect current tests + C4::Context->set_preference('ItemsDeniedRenewal',''); # Generate test biblio my $title = 'Silence in the library'; my ($biblionumber, $biblioitemnumber) = add_biblio($title, 'Moffat, Steven'); @@ -2214,7 +2214,7 @@ subtest 'CanBookBeIssued | is_overdue' => sub { subtest 'ItemsDeniedRenewal preference' => sub { plan tests => 18; - t::lib::Mocks::mock_preference( 'ItemsDeniedRenewal', '' ); + C4::Context->set_preference('ItemsDeniedRenewal',''); $dbh->do('DELETE FROM issues'); $dbh->do('DELETE FROM items'); @@ -2279,7 +2279,7 @@ subtest 'ItemsDeniedRenewal preference' => sub { $idr_rules="withdrawn: [1]"; - t::lib::Mocks::mock_preference( 'ItemsDeniedRenewal', $idr_rules ); + C4::Context->set_preference('ItemsDeniedRenewal',$idr_rules); ( $idr_mayrenew, $idr_error ) = CanBookBeRenewed( $idr_borrower->borrowernumber, $deny_issue->itemnumber ); is( $idr_mayrenew, 0, 'Renewal blocked when 1 rules (withdrawn)' ); @@ -2291,7 +2291,7 @@ subtest 'ItemsDeniedRenewal preference' => sub { $idr_rules="withdrawn: [1]\nitype: [HIDE,INVISIBLE]"; - t::lib::Mocks::mock_preference( 'ItemsDeniedRenewal', $idr_rules ); + C4::Context->set_preference('ItemsDeniedRenewal',$idr_rules); ( $idr_mayrenew, $idr_error ) = CanBookBeRenewed( $idr_borrower->borrowernumber, $deny_issue->itemnumber ); is( $idr_mayrenew, 0, 'Renewal blocked when 2 rules matched (withdrawn, itype)' ); @@ -2303,7 +2303,7 @@ subtest 'ItemsDeniedRenewal preference' => sub { $idr_rules="withdrawn: [1]\nitype: [HIDE,INVISIBLE]\nlocation: [PROC]"; - t::lib::Mocks::mock_preference( 'ItemsDeniedRenewal', $idr_rules ); + C4::Context->set_preference('ItemsDeniedRenewal',$idr_rules); ( $idr_mayrenew, $idr_error ) = CanBookBeRenewed( $idr_borrower->borrowernumber, $deny_issue->itemnumber ); is( $idr_mayrenew, 0, 'Renewal blocked when 3 rules matched (withdrawn, itype, location)' ); @@ -2314,23 +2314,23 @@ subtest 'ItemsDeniedRenewal preference' => sub { is( $idr_error, undef, 'Renewal allowed when 3 rules not matched (withdrawn, itype, location)' ); $idr_rules="itemcallnumber: [NULL]"; - t::lib::Mocks::mock_preference( 'ItemsDeniedRenewal', $idr_rules ); + C4::Context->set_preference('ItemsDeniedRenewal',$idr_rules); ( $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 ); + C4::Context->set_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 ); + C4::Context->set_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 ); + C4::Context->set_preference('ItemsDeniedRenewal',$idr_rules); ( $idr_mayrenew, $idr_error ) = CanBookBeRenewed( $idr_borrower->borrowernumber, $deny_issue->itemnumber ); is( $idr_mayrenew, 0, 'Renewal blocked for empty string when "" in pref' ); @@ -2493,6 +2493,7 @@ subtest 'AddReturn should clear items.onloan for unissued items' => sub { }; $schema->storage->txn_rollback; +C4::Context->clear_syspref_cache(); $cache->clear_from_cache('single_holidays'); sub set_userenv { diff --git a/t/db_dependent/Koha/Config/SysPrefs.t b/t/db_dependent/Koha/Config/SysPrefs.t new file mode 100644 index 0000000..6850f41 --- /dev/null +++ b/t/db_dependent/Koha/Config/SysPrefs.t @@ -0,0 +1,68 @@ +# This file is part of Koha. +# +# Copyright 2018 Koha Development Team +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; +use Test::More tests => 4; + +use t::lib::Mocks; +use t::lib::TestBuilder; +use Koha::Config::SysPrefs; + +my $schema = Koha::Database->new->schema; +$schema->storage->txn_begin; + +my $builder = t::lib::TestBuilder->new; + +my $nb_of_prefs = Koha::Config::SysPrefs->search->count; +my $new_pref = Koha::Config::SysPref->new({ + variable => 'ShouldNotBeDone', + value => 'but a good test?', + options => undef, + explanation => 'just for CRUD sake', + type => 'Free' +})->store; + +is( Koha::Config::SysPrefs->search->count, $nb_of_prefs + 1, 'The 1 pref should have been added' ); +my $retrieved_pref = Koha::Config::SysPrefs->find('ShouldNotBeDone'); +is( $retrieved_pref->value, $new_pref->value, 'Find a pref by variable should return the correct pref' ); + +$retrieved_pref->delete; +is( Koha::Config::SysPrefs->search->count, $nb_of_prefs, 'Delete should have deleted the pref' ); + +subtest 'get_yaml_pref_hash' => sub { + + plan tests => 1; + + my $the_pref = Koha::Config::SysPrefs->find({variable=>'ItemsDeniedRenewal'}); + $the_pref->value(q{ + nulled: [NULL,''] + this: [just_that] + multi_this: [that,another] + }); + + my $expected_hash = { + nulled => [undef,""], + this => ['just_that'], + multi_this => ['that','another'], + }; + my $got_hash = $the_pref->get_yaml_pref_hash(); + is_deeply($got_hash,$expected_hash,"Pref fetched and converted correctly"); + +}; + +$schema->storage->txn_rollback; + -- 2.1.4