From 3e3b7eb9c04d281c5608330717c916e8a5470da7 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Mon, 4 Jun 2018 10:03:40 +0200 Subject: [PATCH] Bug 17530: (QA follow-up) Move may_article_request to ItemType Content-Type: text/plain; charset=utf-8 As requested by QA, we should move may_article_request out of Biblio. For reasons of performance removed the wrapper layer of may_article_request in opac-search. No need to look up all item types. For readability kept the routine in the detail scripts. Note for running ArticleRequests.t: A possible failure on the subtest 'search_limited' is addressed on bug 20866. So you can ignore that one. As long as the subtest for may_article_request passes. Test plan: See previous patches. Signed-off-by: Marcel de Rooy --- Koha/Biblio.pm | 26 -------------------------- Koha/ItemType.pm | 21 +++++++++++++++++++++ opac/opac-ISBDdetail.pl | 2 +- opac/opac-MARCdetail.pl | 3 ++- opac/opac-detail.pl | 2 +- opac/opac-search.pl | 13 +++++-------- t/db_dependent/ArticleRequests.t | 18 +++++------------- 7 files changed, 35 insertions(+), 50 deletions(-) diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index 97edc72..d226304 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -99,32 +99,6 @@ sub can_article_request { return q{}; } -=head3 may_article_request - - Returns true if it is likely possible to make an article request for - a given item type (or the default item type from biblioitems). - - # As class method: - my $boolean = Koha::Biblio->may_article_request({ itemtype => 'BK' }); - # As instance method: - my $boolean = Koha::Biblios->find($biblionumber)->may_article_request; - -=cut - -sub may_article_request { - my ( $class_or_self, $params ) = @_; - return q{} if !C4::Context->preference('ArticleRequests'); - my $itemtype = ref($class_or_self) - ? $class_or_self->itemtype - : $params->{itemtype}; - my $category = $params->{categorycode}; - - my $guess = Koha::IssuingRules->guess_article_requestable_itemtypes({ - $category ? ( categorycode => $category ) : (), - }); - return ( $guess->{ $itemtype // q{} } || $guess->{ '*' } ) ? 1 : q{}; -} - =head3 article_request_type my $type = $biblio->article_request_type( $borrower ); diff --git a/Koha/ItemType.pm b/Koha/ItemType.pm index 24be0d2..3d769a5 100644 --- a/Koha/ItemType.pm +++ b/Koha/ItemType.pm @@ -22,6 +22,7 @@ use Carp; use C4::Koha; use C4::Languages; use Koha::Database; +use Koha::IssuingRules; use Koha::Localizations; use base qw(Koha::Object); @@ -106,6 +107,26 @@ sub can_be_deleted { return $nb_items + $nb_biblioitems == 0 ? 1 : 0; } +=head3 may_article_request + + Returns true if it is likely possible to make an article request for + this item type. + Optional parameter: categorycode (for patron). + +=cut + +sub may_article_request { + my ( $self, $params ) = @_; + return q{} if !C4::Context->preference('ArticleRequests'); + my $itemtype = $self->itemtype; + my $category = $params->{categorycode}; + + my $guess = Koha::IssuingRules->guess_article_requestable_itemtypes({ + $category ? ( categorycode => $category ) : (), + }); + return ( $guess->{ $itemtype // q{} } || $guess->{ '*' } ) ? 1 : q{}; +} + =head3 type =cut diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl index cdbd37b..fe69564 100755 --- a/opac/opac-ISBDdetail.pl +++ b/opac/opac-ISBDdetail.pl @@ -220,7 +220,7 @@ if (my $search_for_title = C4::Context->preference('OPACSearchForTitleIn')){ if( C4::Context->preference('ArticleRequests') ) { my $artreqpossible = $patron ? $biblio->can_article_request( $patron ) - : $biblio->may_article_request; + : Koha::ItemTypes->find($biblio->itemtype)->may_article_request; $template->param( artreqpossible => $artreqpossible ); } diff --git a/opac/opac-MARCdetail.pl b/opac/opac-MARCdetail.pl index 926b70b..d438036 100755 --- a/opac/opac-MARCdetail.pl +++ b/opac/opac-MARCdetail.pl @@ -60,6 +60,7 @@ use List::MoreUtils qw( any uniq ); use Koha::Biblios; use Koha::IssuingRules; use Koha::Items; +use Koha::ItemTypes; use Koha::Patrons; use Koha::RecordProcessor; @@ -352,7 +353,7 @@ if (my $search_for_title = C4::Context->preference('OPACSearchForTitleIn')){ if( C4::Context->preference('ArticleRequests') ) { my $artreqpossible = $patron ? $biblio->can_article_request( $patron ) - : $biblio->may_article_request; + : Koha::ItemTypes->find($biblio->itemtype)->may_article_request; $template->param( artreqpossible => $artreqpossible ); } diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index 2b80567..90efac9 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -760,7 +760,7 @@ if( C4::Context->preference('ArticleRequests') ) { my $patron = $borrowernumber ? Koha::Patrons->find($borrowernumber) : undef; my $artreqpossible = $patron ? $biblio->can_article_request( $patron ) - : $biblio->may_article_request; + : Koha::ItemTypes->find($biblio->itemtype)->may_article_request; $template->param( artreqpossible => $artreqpossible ); } diff --git a/opac/opac-search.pl b/opac/opac-search.pl index debba77..78cc753 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -642,10 +642,10 @@ for (my $i=0;$i<@servers;$i++) { } $hits = 0 unless @newresults; - my $categorycode; # needed for may_article_request - if( $borrowernumber && C4::Context->preference('ArticleRequests') ) { - my $patron = Koha::Patrons->find( $borrowernumber ); - $categorycode = $patron ? $patron->categorycode : undef; + my $art_req_itypes; + if( C4::Context->preference('ArticleRequests') ) { + my $patron = $borrowernumber ? Koha::Patrons->find( $borrowernumber ) : undef; + $art_req_itypes = Koha::IssuingRules->guess_article_requestable_itemtypes({ $patron ? ( categorycode => $patron->categorycode ) : () }); } foreach my $res (@newresults) { @@ -705,10 +705,7 @@ for (my $i=0;$i<@servers;$i++) { } # BZ17530: 'Intelligent' guess if result can be article requested - $res->{artreqpossible} = Koha::Biblio->may_article_request({ - categorycode => $categorycode, - itemtype => $res->{itemtype}, - }); + $res->{artreqpossible} = ( $art_req_itypes->{ $res->{itemtype} // q{} } || $art_req_itypes->{ '*' } ) ? 1 : q{}; } if ($results_hashref->{$server}->{"hits"}){ diff --git a/t/db_dependent/ArticleRequests.t b/t/db_dependent/ArticleRequests.t index 9c14a24..2587db6 100755 --- a/t/db_dependent/ArticleRequests.t +++ b/t/db_dependent/ArticleRequests.t @@ -223,7 +223,7 @@ subtest 'search_limited' => sub { }; subtest 'may_article_request' => sub { - plan tests => 6; + plan tests => 3; # mocking t::lib::Mocks::mock_preference('ArticleRequests', 1); @@ -233,18 +233,10 @@ subtest 'may_article_request' => sub { 'PT' => { 'BK' => 1 }, }); - # tests for class method call - is( Koha::Biblio->may_article_request({ itemtype => 'CR' }), 1, 'SER/* should be true' ); - is( Koha::Biblio->may_article_request({ itemtype => 'CR', categorycode => 'S' }), 1, 'SER/S should be true' ); - is( Koha::Biblio->may_article_request({ itemtype => 'CR', categorycode => 'PT' }), '', 'SER/PT should be false' ); - - # tests for instance method call - my $builder = t::lib::TestBuilder->new; - my $biblio = $builder->build_object({ class => 'Koha::Biblios' }); - my $biblioitem = $builder->build_object({ class => 'Koha::Biblioitems', value => { biblionumber => $biblio->biblionumber, itemtype => 'BK' }}); - is( $biblio->may_article_request, '', 'BK/* false' ); - is( $biblio->may_article_request({ categorycode => 'S' }), 1, 'BK/S true' ); - is( $biblio->may_article_request({ categorycode => 'PT' }), 1, 'BK/PT true' ); + my $itemtype = Koha::ItemTypes->find('CR') // Koha::ItemType->new({ itemtype => 'CR' })->store; + is( $itemtype->may_article_request, 1, 'SER/* should be true' ); + is( $itemtype->may_article_request({ categorycode => 'S' }), 1, 'SER/S should be true' ); + is( $itemtype->may_article_request({ categorycode => 'PT' }), '', 'SER/PT should be false' ); # Cleanup $cache->clear_from_cache( Koha::IssuingRules::GUESSED_ITEMTYPES_KEY ); -- 2.1.4