Bugzilla – Attachment 80332 Details for
Bug 15494
Block renewals by arbitrary item values
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 15494: (follow-up) Move yaml syspref code to its own sub in SySpref object
Bug-15494-follow-up-Move-yaml-syspref-code-to-its-.patch (text/plain), 11.51 KB, created by
Nick Clemens (kidclamp)
on 2018-10-10 11:00:56 UTC
(
hide
)
Description:
Bug 15494: (follow-up) Move yaml syspref code to its own sub in SySpref object
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2018-10-10 11:00:56 UTC
Size:
11.51 KB
patch
obsolete
>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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=80332">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 15494
:
63700
|
63701
|
63702
|
63703
|
67886
|
67887
|
67888
|
67889
|
68146
|
68147
|
68148
|
68149
|
70295
|
70629
|
70630
|
70631
|
70632
|
71129
|
71130
|
71131
|
71132
|
71133
|
71134
|
71135
|
71136
|
71137
|
71138
|
79249
|
79250
|
79251
|
79252
|
79253
|
79254
|
79293
|
80327
|
80328
|
80329
|
80330
|
80331
|
80332
|
80333
|
81411
|
81412
|
81413
|
81414
|
81415
|
81416
|
81417