From 4f03b5318ea9c545ab76ac37bb1308dae3c85dda Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
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 <http://koha-community.org/>
+
+Nick Clemens <nick@bywatersolutions.com>
+
+=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 <http://www.gnu.org/licenses>.
+
+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