From 70c6266081a566149c7b3b49c95958a31417d435 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 15 Mar 2018 12:04:30 +0100 Subject: [PATCH] Bug 17530: (QA follow-up) Replace our variable by cached value Instead of saving the state locally in a variable during Plack lifetime, we move the saved hash to the cache. We clear the key when we enter smart-rules.pl. This makes a change in circ rules immediately effective. Test plan: Run the modified tests. Signed-off-by: Marcel de Rooy Signed-off-by: Chris Cormack Signed-off-by: Nick Clemens --- Koha/IssuingRules.pm | 9 ++++++--- admin/smart-rules.pl | 4 ++++ t/db_dependent/ArticleRequests.t | 8 +++++--- .../IssuingRules/guess_article_requestable_itemtypes.t | 7 +++++-- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/Koha/IssuingRules.pm b/Koha/IssuingRules.pm index 642c55da33..c72dd46dd9 100644 --- a/Koha/IssuingRules.pm +++ b/Koha/IssuingRules.pm @@ -21,11 +21,14 @@ package Koha::IssuingRules; use Modern::Perl; use Koha::Database; +use Koha::Caches; use Koha::IssuingRule; use base qw(Koha::Objects); +use constant GUESSED_ITEMTYPES_KEY => 'Koha_IssuingRules_last_guess'; + =head1 NAME Koha::IssuingRules - Koha IssuingRule Object set class @@ -157,13 +160,13 @@ sub article_requestable_rules { =cut -our $last_article_requestable_guesses; # used during Plack life time - sub guess_article_requestable_itemtypes { my ( $class_or_self, $params ) = @_; my $category = $params->{categorycode}; return {} if !C4::Context->preference('ArticleRequests'); + my $cache = Koha::Caches->get_instance; + my $last_article_requestable_guesses = $cache->get_from_cache(GUESSED_ITEMTYPES_KEY); my $key = $category || '*'; return $last_article_requestable_guesses->{$key} if $last_article_requestable_guesses && exists $last_article_requestable_guesses->{$key}; @@ -176,7 +179,7 @@ sub guess_article_requestable_itemtypes { foreach my $rule ( $rules->as_list ) { $res->{ $rule->itemtype } = 1; } - $last_article_requestable_guesses->{$key} = $res; + $cache->set_in_cache(GUESSED_ITEMTYPES_KEY, $res); return $res; } diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index eb4ef7b54f..d87d8e5451 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -33,6 +33,7 @@ use Koha::RefundLostItemFeeRule; use Koha::RefundLostItemFeeRules; use Koha::Libraries; use Koha::Patron::Categories; +use Koha::Caches; my $input = CGI->new; my $dbh = C4::Context->dbh; @@ -64,6 +65,9 @@ $branch = '*' if $branch eq 'NO_LIBRARY_SET'; my $op = $input->param('op') || q{}; my $language = C4::Languages::getlanguage(); +my $cache = Koha::Caches->get_instance; +$cache->clear_from_cache( Koha::IssuingRules::GUESSED_ITEMTYPES_KEY ); + if ($op eq 'delete') { my $itemtype = $input->param('itemtype'); my $categorycode = $input->param('categorycode'); diff --git a/t/db_dependent/ArticleRequests.t b/t/db_dependent/ArticleRequests.t index d38bf02d43..c0b92b612f 100755 --- a/t/db_dependent/ArticleRequests.t +++ b/t/db_dependent/ArticleRequests.t @@ -30,6 +30,7 @@ use Koha::Notice::Messages; use Koha::Patron; use Koha::Library::Group; use Koha::IssuingRules; +use Koha::Caches; BEGIN { use_ok('Koha::ArticleRequest'); @@ -40,6 +41,7 @@ BEGIN { my $schema = Koha::Database->new()->schema(); $schema->storage->txn_begin(); my $builder = t::lib::TestBuilder->new; +our $cache = Koha::Caches->get_instance; my $dbh = C4::Context->dbh; $dbh->{RaiseError} = 1; @@ -223,11 +225,11 @@ subtest 'may_article_request' => sub { # mocking t::lib::Mocks::mock_preference('ArticleRequests', 1); - $Koha::IssuingRules::last_article_requestable_guesses = { + $cache->set_in_cache( Koha::IssuingRules::GUESSED_ITEMTYPES_KEY, { '*' => { 'CR' => 1 }, 'S' => { '*' => 1 }, 'PT' => { 'BK' => 1 }, - }; + }); # tests for class method call is( Koha::Biblio->may_article_request({ itemtype => 'CR' }), 1, 'SER/* should be true' ); @@ -243,7 +245,7 @@ subtest 'may_article_request' => sub { is( $biblio->may_article_request({ categorycode => 'PT' }), 1, 'BK/PT true' ); # Cleanup - $Koha::IssuingRules::last_article_requestable_guesses = undef; + $cache->clear_from_cache( Koha::IssuingRules::GUESSED_ITEMTYPES_KEY ); }; $schema->storage->txn_rollback(); diff --git a/t/db_dependent/Koha/IssuingRules/guess_article_requestable_itemtypes.t b/t/db_dependent/Koha/IssuingRules/guess_article_requestable_itemtypes.t index 71c6c66b15..55529181f3 100644 --- a/t/db_dependent/Koha/IssuingRules/guess_article_requestable_itemtypes.t +++ b/t/db_dependent/Koha/IssuingRules/guess_article_requestable_itemtypes.t @@ -7,15 +7,18 @@ use t::lib::Mocks; use t::lib::TestBuilder; use Koha::Database; use Koha::IssuingRules; +use Koha::Caches; my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; our $builder = t::lib::TestBuilder->new; +our $cache = Koha::Caches->get_instance; subtest 'guess_article_requestable_itemtypes' => sub { plan tests => 12; t::lib::Mocks::mock_preference('ArticleRequests', 1); + $cache->clear_from_cache( Koha::IssuingRules::GUESSED_ITEMTYPES_KEY ); Koha::IssuingRules->delete; my $itype1 = $builder->build_object({ class => 'Koha::ItemTypes' }); my $itype2 = $builder->build_object({ class => 'Koha::ItemTypes' }); @@ -51,7 +54,7 @@ subtest 'guess_article_requestable_itemtypes' => sub { # Change the rules $rule2->itemtype('*')->store; - $Koha::IssuingRules::last_article_requestable_guesses = {}; + $cache->clear_from_cache( Koha::IssuingRules::GUESSED_ITEMTYPES_KEY ); $res = Koha::IssuingRules->guess_article_requestable_itemtypes; is( $res->{'*'}, 1, 'Item type * seems permitted' ); is( $res->{$itype1->itemtype}, 1, 'Item type 1 seems permitted' ); @@ -61,7 +64,7 @@ subtest 'guess_article_requestable_itemtypes' => sub { is( $res->{$itype1->itemtype}, 1, 'Item type 1 seems permitted' ); is( $res->{$itype2->itemtype}, undef, 'Item type 2 seems not permitted' ); - $Koha::IssuingRules::last_article_requestable_guesses = {}; + $cache->clear_from_cache( Koha::IssuingRules::GUESSED_ITEMTYPES_KEY ); }; $schema->storage->txn_rollback; -- 2.39.5