Bug 14385: (QA follow-up) Additional changes and fixes

[1] searchResults: second my $interface can be removed: unused
[2] call of getitemtypeimagelocation on L2119 needs interface key
[3] ISBDdetail: No need to find patron again (line 182 vs 84)
[4] opac-search: No need to find patron twice (657 and 631)
[5] tabs on line 2220 of C4/Search.pm (qa tools warn)
[6] Ugly hack to overcome "Undefined subroutine &C4::Items::ModZebra"
    by loading C4::Items before C4::Biblio when running tests
    Koha/BiblioUtils/Iterator.t and Labels/t_Label.t.
    This is a more general problem that needs attention somewhere else.
    It seems that Biblio.pm is one of the suspects.
[7] This patch set makes Search.t crash/fail with me. Note that without
    these patches Search.t still passed! Why o why..
    A little debugging pointed me to a missing MPL branch (aarg).
    Adding the simple test on the result of Libraries->find in
    C4::Biblio::GetAuthorisedValueDesc made the test continue.
[8] Resolve: Variable "$borcat" is not available at opac-detail.pl line 246
    Lexical $borcat cannot be used in sub searchAgain in opac-detail.pl
    under Plack. Must be defined with our (or passed as argument).
[9] Resolve crash on TWO serious typos in opac-basket on ONE line:
        Koha::Patron->find({ borrowernumber -> $borrowernumber })
    Yeah: find is in Koha::Patrons and we need => !!
    No need to pass a hash to find method btw for a pk value.
[10] Serious bugfixing here. Add List::Util to opac-basket.
    Can't locate object method "none" via package "1".
    You can't test everything :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
After this longer list I renamed Final to Additional in the patch title :)

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

diff --git a/C4/Biblio.pm b/C4/Biblio.pm
index a8a85f2..647d1d0 100644 (file)
--- a/C4/Biblio.pm
+++ b/C4/Biblio.pm
@@ -1548,7 +1548,8 @@ sub GetAuthorisedValueDesc {
 
         #---- branch
         if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "branches" ) {
-            return Koha::Libraries->find($value)->branchname;
+            my $branch = Koha::Libraries->find($value);
+            return $branch? $branch->branchname: q{};
         }
 
         #---- itemtypes

diff --git a/C4/Search.pm b/C4/Search.pm
index 1e4ae38..5f8250d 100644 (file)
--- a/C4/Search.pm
+++ b/C4/Search.pm
@@ -2116,7 +2116,7 @@ sub searchResults {
                 $onloan_items->{$key}->{itemcallnumber} = $item->{itemcallnumber};
                 $onloan_items->{$key}->{description}    = $item->{description};
                 $onloan_items->{$key}->{imageurl} =
-                  getitemtypeimagelocation( $search_context, $itemtypes{ $item->{itype} }->{imageurl} );
+                  getitemtypeimagelocation( $search_context->{'interface'}, $itemtypes{ $item->{itype} }->{imageurl} );
 
                 # if something's checked out and lost, mark it as 'long overdue'
                 if ( $item->{itemlost} ) {
@@ -2215,8 +2215,8 @@ sub searchResults {
                 else {
                     $can_place_holds = 1;
                     $available_count++;
-                                       $available_items->{$prefix}->{count}++ if $item->{$hbranch};
-                                       foreach (qw(branchname itemcallnumber description)) {
+                    $available_items->{$prefix}->{count}++ if $item->{$hbranch};
+                    foreach (qw(branchname itemcallnumber description)) {
                         $available_items->{$prefix}->{$_} = $item->{$_};
                     }
                     $available_items->{$prefix}->{location} = $shelflocations->{ $item->{location} };
@@ -2246,7 +2246,6 @@ sub searchResults {
 
         # XSLT processing of some stuff
         # we fetched the sysprefs already before the loop through all retrieved record!
-        my $interface = $search_context->{'interface'} eq 'opac' ? 'OPAC' : '';
         if (!$scan && $xslfile) {
             $oldbiblio->{XSLTResultsRecord} = XSLTParse4Display($oldbiblio->{biblionumber}, $marcrecord, $xslsyspref, 1, \@hiddenitems, $sysxml, $xslfile, $lang);
         }

diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl
index 1cfbf00..ed39f72 100755 (executable)
--- a/opac/opac-ISBDdetail.pl
+++ b/opac/opac-ISBDdetail.pl
@@ -78,11 +78,10 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
     }
 );
 
+my $patron = Koha::Patrons->find( $loggedinuser );
 my $borcat = q{};
-if ( C4::Context->preference('OpacHiddenItemsExceptions') ) {
-    # we need to fetch the borrower info here, so we can pass the category
-    my $patron = Koha::Patrons->find( { borrowernumber => $loggedinuser } );
-    $borcat = $patron ? $patron->categorycode : $borcat;
+if ( $patron && C4::Context->preference('OpacHiddenItemsExceptions') ) {
+    $borcat = $patron->categorycode;
 }
 
 my $record = GetMarcBiblio({
@@ -179,7 +178,6 @@ my $res = GetISBDView({
 });
 
 my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search_with_localization->unblessed } };
-my $patron = Koha::Patrons->find( $loggedinuser );
 for my $itm (@all_items) {
     my $item = Koha::Items->find( $itm->{itemnumber} );
     $norequests = 0

diff --git a/opac/opac-basket.pl b/opac/opac-basket.pl
index fd48dcf..d66fd01 100755 (executable)
--- a/opac/opac-basket.pl
+++ b/opac/opac-basket.pl
@@ -18,6 +18,8 @@
 use Modern::Perl;
 
 use CGI qw ( -utf8 );
+use List::Util qw/none/; # well just one :)
+
 use C4::Koha;
 use C4::Biblio;
 use C4::Items;
@@ -59,8 +61,8 @@ if (C4::Context->preference('TagsEnabled')) {
 my $borcat = q{};
 if ( C4::Context->preference('OpacHiddenItemsExceptions') ) {
     # we need to fetch the borrower info here, so we can pass the category
-    my $borrower = Koha::Patron->find( { borrowernumber -> $borrowernumber } );
-    $borcat = $borrower ? $borrower->categorycode : $borcat;
+    my $patron = Koha::Patrons->find($borrowernumber);
+    $borcat = $patron ? $patron->categorycode : $borcat;
 }
 
 my $record_processor = Koha::RecordProcessor->new({ filters => 'ViewPolicy' });

diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl
index 673282c..d81a2a1 100755 (executable)
--- a/opac/opac-detail.pl
+++ b/opac/opac-detail.pl
@@ -85,7 +85,7 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
 my @all_items = GetItemsInfo($biblionumber);
 my @hiddenitems;
 my $patron = Koha::Patrons->find( $borrowernumber );
-my $borcat= q{};
+our $borcat= q{};
 if ( C4::Context->preference('OpacHiddenItemsExceptions') ) {
     $borcat = $patron ? $patron->categorycode : q{};
 }

diff --git a/opac/opac-search.pl b/opac/opac-search.pl
index de72a8b..b4010a9 100755 (executable)
--- a/opac/opac-search.pl
+++ b/opac/opac-search.pl
@@ -125,6 +125,7 @@ else {
     authnotrequired => ( C4::Context->preference("OpacPublic") ? 1 : 0 ),
     }
 );
+my $patron = Koha::Patrons->find( $borrowernumber );
 
 my $lang = C4::Languages::getlanguage($cgi);
 
@@ -627,8 +628,6 @@ my @sup_results_array;
 my $search_context = {};
 $search_context->{'interface'} = 'opac';
 if (C4::Context->preference('OpacHiddenItemsExceptions')){
-    # we need to fetch the borrower info here, so we can pass the category
-    my $patron = Koha::Patrons->find( { borrowernumber => $borrowernumber } );
     $search_context->{'category'} = $patron ? $patron->categorycode : q{};
 }
 
@@ -655,7 +654,6 @@ for (my $i=0;$i<@servers;$i++) {
 
         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 ) : () });
         }
 

diff --git a/t/db_dependent/Koha/BiblioUtils/Iterator.t b/t/db_dependent/Koha/BiblioUtils/Iterator.t
index ac11cf7..9009b00 100644 (file)
--- a/t/db_dependent/Koha/BiblioUtils/Iterator.t
+++ b/t/db_dependent/Koha/BiblioUtils/Iterator.t
@@ -22,8 +22,8 @@ use Test::More tests => 3;
 use_ok('Koha::BiblioUtils');
 use_ok('Koha::BiblioUtils::Iterator');
 
-use C4::Biblio;
 use C4::Items;
+use C4::Biblio;
 use DBI;
 use t::lib::TestBuilder;
 use t::lib::Mocks;

diff --git a/t/db_dependent/Labels/t_Label.t b/t/db_dependent/Labels/t_Label.t
index 4f60b07..2d44b44 100644 (file)
--- a/t/db_dependent/Labels/t_Label.t
+++ b/t/db_dependent/Labels/t_Label.t
@@ -26,8 +26,8 @@ use MARC::Record;
 use MARC::Field;
 use Data::Dumper;
 
-use C4::Biblio;
 use C4::Items;
+use C4::Biblio;
 use C4::Labels::Layout;
 
 use Koha::Database;