Bug 17530: (QA follow-up) Move may_article_request to ItemType

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 <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm
index 7a60b41..1dca4d9 100644 (file)
--- a/Koha/Biblio.pm
+++ b/Koha/Biblio.pm
@@ -169,32 +169,6 @@ sub can_be_transferred {
     return 0;
 }
 
-=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 (file)
--- 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 (executable)
--- 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 (executable)
--- 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 e0fbb90..6f10c6b 100755 (executable)
--- 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 b36aaaf..ce2cefc 100755 (executable)
--- 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 c0b92b6..65b3288 100755 (executable)
--- a/t/db_dependent/ArticleRequests.t
+++ b/t/db_dependent/ArticleRequests.t
@@ -221,7 +221,7 @@ subtest 'search_limited' => sub {
 };
 
 subtest 'may_article_request' => sub {
-    plan tests => 6;
+    plan tests => 3;
 
     # mocking
     t::lib::Mocks::mock_preference('ArticleRequests', 1);
@@ -231,18 +231,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 );