From 9d5dd543b39d063e1a1fa0ccd20216b34aee972b Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 12 Oct 2018 07:54:24 +0200 Subject: [PATCH] Bug 14385: (QA follow-up) Additional changes and fixes Content-Type: text/plain; charset=utf-8 [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 After this longer list I renamed Final to Additional in the patch title :) --- C4/Biblio.pm | 3 ++- C4/Search.pm | 7 +++---- opac/opac-ISBDdetail.pl | 8 +++----- opac/opac-basket.pl | 6 ++++-- opac/opac-detail.pl | 2 +- opac/opac-search.pl | 4 +--- t/db_dependent/Koha/BiblioUtils/Iterator.t | 2 +- t/db_dependent/Labels/t_Label.t | 2 +- 8 files changed, 16 insertions(+), 18 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 153241f..5b10c00 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -1564,7 +1564,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 --- 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 --- 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 --- 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 56ebf62..3792546 100755 --- 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 --- 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 --- 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 --- 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; -- 2.1.4