From 970445fdad120d46101e0234a852f2ed56a10249 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Mon, 12 Jul 2021 13:58:51 +0000 Subject: [PATCH] Bug 28702: Make buildKohaItemsNamespace take an array If we fetch some of the authorised values and before hand we can reduce the amount of work needed in this routine. We stil require queries for pending holds and transfers, but these are lighter than fetching the items To test: 1 - Perform a search on the OPAC 2 - Add the results to a list 3 - Load the list several times and use developer tools (F12) to view the time to load in the network tab 4 - Repeat a search several times and use developer tools (F12) to view the time to load in the network tab 5 - Record the times noted above 6 - Apply patch 7 - Repeat the search and list view and compare to before the patch 8 - prove -v t/db_dependent/XSLT.t --- C4/Reserves.pm | 13 +++- C4/Search.pm | 29 ++++--- C4/XSLT.pm | 83 +++++++------------- opac/opac-shelves.pl | 18 +++++ t/db_dependent/XSLT.t | 177 ++++++++++-------------------------------- 5 files changed, 119 insertions(+), 201 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index d5e01f283d..b81c68acd5 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -804,13 +804,20 @@ sub GetReserveStatus { my $dbh = C4::Context->dbh; - my ($sth, $found, $priority); + my ($sth, $found, $priority, $pending); if ( $itemnumber ) { - $sth = $dbh->prepare("SELECT found, priority FROM reserves WHERE itemnumber = ? order by priority LIMIT 1"); + $sth = $dbh->prepare(q{ + SELECT found, priority, IF( tmp_holdsqueue.itemnumber IS NULL, 0, 1) AS pending + FROM reserves + LEFT JOIN tmp_holdsqueue USING (itemnumber) + WHERE itemnumber = ? order by priority LIMIT 1 + }); $sth->execute($itemnumber); - ($found, $priority) = $sth->fetchrow_array; + ($found, $priority, $pending) = $sth->fetchrow_array; } + return 'Pending' if $pending; + if(defined $found) { return 'Waiting' if $found eq 'W' and $priority == 0; return 'Processing' if $found eq 'P'; diff --git a/C4/Search.pm b/C4/Search.pm index 5abf66b842..9f9af157d4 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -1641,15 +1641,16 @@ sub searchResults { #Build branchnames hash my %branches = map { $_->branchcode => $_->branchname } Koha::Libraries->search({}, { order_by => 'branchname' })->as_list; -# FIXME - We build an authorised values hash here, using the default framework -# though it is possible to have different authvals for different fws. - + my $description = $is_opac ? 'opac_description' : 'lib'; my $shelflocations = - { map { $_->{authorised_value} => $_->{lib} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => '', kohafield => 'items.location' } ) }; + { map { $_->{authorised_value} => $_->{$description} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => '', kohafield => 'items.location' } ) }; + my $ccodes = { map { $_->{authorised_value} => $_->{$description} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => "", kohafield => 'items.ccode' } ) }; # get notforloan authorised value list (see $shelflocations FIXME) my $av = Koha::MarcSubfieldStructures->search({ frameworkcode => '', kohafield => 'items.notforloan', authorised_value => [ -and => {'!=' => undef }, {'!=' => ''}] }); my $notforloan_authorised_value = $av->count ? $av->next->authorised_value : undef; + my $notforloan_descriptions = { map { $_->{authorised_value} => $_->{$description} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { kohafield => 'items.notforloan' } ) }; + #Get itemtype hash my $itemtypes = Koha::ItemTypes->search_with_localization; @@ -1793,6 +1794,7 @@ sub searchResults { my @hiddenitems; # hidden itemnumbers based on OpacHiddenItems syspref # loop through every item + my @items = (); foreach my $field (@fields) { my $item; @@ -1830,6 +1832,8 @@ sub searchResults { elsif ($item->{$otherbranch}) { # Last resort $item->{'branchname'} = $branches{$item->{$otherbranch}}; } + $item->{homebranch} = $branches{ $item->{homebranch} } if defined $item->{homebranch}; + $item->{holdingbranch} = $branches{ $item->{holdingbranch} } if defined $item->{holdingbranch}; my $prefix = ( $item->{$hbranch} ? $item->{$hbranch} . '--' : q{} ) @@ -1837,6 +1841,12 @@ sub searchResults { . ( $item->{itype} ? $item->{itype} : q{} ) . ( $item->{itemcallnumber} ? $item->{itemcallnumber} : q{} ); # For each grouping of items (onloan, available, unavailable), we build a key to store relevant info about that item + my $itemtype = C4::Context->preference("item-level_itypes")? $item->{itype}: $oldbiblio->{itemtype}; + $item->{itemtype} = $itemtype; + $item->{location} = $item->{location} && exists $shelflocations->{$item->{location}} ? $shelflocations->{$item->{location}} : $item->{location} ; + $item->{notforloan_description} = exists $notforloan_descriptions->{ $item->{notforloan} } ? $notforloan_descriptions->{ $item->{notforloan} } : "Not for loan"; + $item->{ccode_description} = defined $item->{ccode} && exists $ccodes->{ $item->{ccode} } ? $ccodes->{ $item->{ccode} } : $item->{ccode}; + $item->{notforloan_itemtype} = defined $itemtype && exists $itemtypes{ $itemtype } ? $itemtypes{ $itemtype }->{notforloan} : undef; if ( $item->{onloan} and $logged_in_user and !( $patron_category_hide_lost_items and $item->{itemlost} ) ) @@ -1846,7 +1856,7 @@ sub searchResults { $onloan_items->{$key}->{due_date} = $item->{onloan}; $onloan_items->{$key}->{count}++ if $item->{$hbranch}; $onloan_items->{$key}->{branchname} = $item->{branchname}; - $onloan_items->{$key}->{location} = $shelflocations->{ $item->{location} } if $item->{location}; + $onloan_items->{$key}->{location} = $item->{location} if $item->{location}; $onloan_items->{$key}->{itemcallnumber} = $item->{itemcallnumber}; $onloan_items->{$key}->{description} = $item->{description}; $onloan_items->{$key}->{imageurl} = @@ -1862,7 +1872,6 @@ sub searchResults { # items not on loan, but still unavailable ( lost, withdrawn, damaged ) else { - my $itemtype = C4::Context->preference("item-level_itypes")? $item->{itype}: $oldbiblio->{itemtype}; $item->{notforloan} = 1 if !$item->{notforloan} && $itemtype && $itemtypes{ $itemtype }->{notforloan}; @@ -1939,7 +1948,7 @@ sub searchResults { $other_items->{$key}->{onhold} = ($reservestatus) ? 1 : 0; $other_items->{$key}->{notforloan} = GetAuthorisedValueDesc('','',$item->{notforloan},'','',$notforloan_authorised_value) if $notforloan_authorised_value and $item->{notforloan}; $other_items->{$key}->{count}++ if $item->{$hbranch}; - $other_items->{$key}->{location} = $shelflocations->{ $item->{location} } if $item->{location}; + $other_items->{$key}->{location} = $item->{location} if $item->{location}; $other_items->{$key}->{description} = $item->{description}; $other_items->{$key}->{imageurl} = getitemtypeimagelocation( $search_context->{'interface'}, $itemtypes{ $item->{itype}//q{} }->{imageurl} ); } @@ -1950,10 +1959,11 @@ sub searchResults { foreach (qw(branchname itemcallnumber description)) { $available_items->{$prefix}->{$_} = $item->{$_}; } - $available_items->{$prefix}->{location} = $shelflocations->{ $item->{location} } if $item->{location}; + $available_items->{$prefix}->{location} = $item->{location} if $item->{location}; $available_items->{$prefix}->{imageurl} = getitemtypeimagelocation( $search_context->{'interface'}, $itemtypes{ $item->{itype}//q{} }->{imageurl} ); } } + push @items, $item; } # notforloan, item level and biblioitem level # if all items are hidden, do not show the record @@ -1996,7 +2006,8 @@ sub searchResults { ), fix_amps => 1, hidden_items => \@hiddenitems, - xslt_variables => $xslt_variables + xslt_variables => $xslt_variables, + items => \@items } ); } diff --git a/C4/XSLT.pm b/C4/XSLT.pm index 0fb7be0d6c..053b674fa3 100644 --- a/C4/XSLT.pm +++ b/C4/XSLT.pm @@ -250,7 +250,7 @@ sub XSLTParse4Display { my $fixamps = $params->{fix_amps}; my $hidden_items = $params->{hidden_items} || []; my $variables = $params->{xslt_variables}; - my $items_rs = $params->{items_rs}; + my $items = $params->{items}; die "Mandatory \$params->{xsl_syspref} was not provided, called with biblionumber $params->{biblionumber}" if not defined $params->{xsl_syspref}; @@ -263,7 +263,7 @@ sub XSLTParse4Display { if ( $xslsyspref eq "OPACXSLTDetailsDisplay" || $xslsyspref eq "XSLTDetailsDisplay" || $xslsyspref eq "XSLTResultsDisplay" ) { $itemsxml = ""; #We don't use XSLT for items display on these pages } else { - $itemsxml = buildKohaItemsNamespace($biblionumber, $hidden_items, $items_rs); + $itemsxml = buildKohaItemsNamespace($items); } my $xmlrecord = $record->as_xml(C4::Context->preference('marcflavour')); @@ -310,98 +310,73 @@ sub XSLTParse4Display { =head2 buildKohaItemsNamespace - my $items_xml = buildKohaItemsNamespace( $biblionumber, [ $hidden_items, $items ] ); + my $items_xml = buildKohaItemsNamespace( $items ); -Returns XML for items. It accepts two optional parameters: -- I<$hidden_items>: An arrayref of itemnumber values, for items that should be hidden -- I<$items>: A Koha::Items resultset, for the items to be returned - -If both parameters are passed, I<$items> is used as the basis resultset, and I<$hidden_items> -are filtered out of it. +Returns XML for items. It requires one parameter: +- I<$items>: An arrayref of item hashes, for the items to be returned, hidden items should be filtered before calling this routine Is only used in this module currently. =cut sub buildKohaItemsNamespace { - my ($biblionumber, $hidden_items, $items_rs) = @_; - - $hidden_items ||= []; - - my $query = {}; - $query = { 'me.itemnumber' => { not_in => $hidden_items } } - if $hidden_items; - - unless ( $items_rs && ref($items_rs) eq 'Koha::Items' ) { - $query->{'me.biblionumber'} = $biblionumber; - $items_rs = Koha::Items->new; - } + my ($items) = @_; - my $items = $items_rs->search( $query, { prefetch => [ 'branchtransfers', 'reserves' ] } ); - - my $shelflocations = - { map { $_->{authorised_value} => $_->{opac_description} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => "", kohafield => 'items.location' } ) }; - my $ccodes = - { map { $_->{authorised_value} => $_->{opac_description} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => "", kohafield => 'items.ccode' } ) }; - - my %branches = map { $_->branchcode => $_->branchname } Koha::Libraries->search({}, { order_by => 'branchname' })->as_list; - - my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search->unblessed } }; my $xml = ''; - my %descs = map { $_->{authorised_value} => $_ } Koha::AuthorisedValues->get_descriptions_by_koha_field( { kohafield => 'items.notforloan' } ); my $ref_status = C4::Context->preference('Reference_NFL_Statuses') || '1|2'; - while ( my $item = $items->next ) { + for my $item ( @$items ) { my $status; my $substatus = ''; my $recalls_count; if ( C4::Context->preference('UseRecalls') ) { - $recalls_count = Koha::Recalls->search({ item_id => $item->itemnumber, status => 'waiting' })->count; + $recalls_count = Koha::Recalls->search({ item_id => $item->{itemnumber}, status => 'waiting' })->count; } + my $reservestatus = C4::Reserves::GetReserveStatus( $item->{itemnumber} ); + my $transfer = C4::Circulation::GetTransfers($item->{itemnumber}); + if ($recalls_count) { # recalls take priority over holds $status = 'other'; $substatus = 'Recall waiting'; } - elsif ( $item->has_pending_hold ) { + elsif ($reservestatus eq 'Pending' ) { $status = 'other'; $substatus = 'Pending hold'; } - elsif ( $item->holds->waiting->count ) { + elsif ( $reservestatus eq 'Waiting' ) { $status = 'other'; - $substatus = 'Hold waiting'; + $substatus = 'Waiting'; } - elsif ($item->get_transfer) { + elsif ($transfer) { $status = 'other'; $substatus = 'In transit'; } - elsif ($item->damaged) { + elsif ($item->{damaged}) { $status = 'other'; $substatus = "Damaged"; } - elsif ($item->itemlost) { + elsif ($item->{itemlost}) { $status = 'other'; $substatus = "Lost"; } - elsif ( $item->withdrawn) { + elsif ( $item->{withdrawn}) { $status = 'other'; $substatus = "Withdrawn"; } - elsif ($item->onloan) { + elsif ($item->{onloan}) { $status = 'other'; $substatus = "Checked out"; } - elsif ( $item->notforloan ) { - $status = $item->notforloan =~ /^($ref_status)$/ + elsif ( $item->{notforloan} ) { + $status = $item->{notforloan} =~ /^($ref_status)$/ ? "reference" : "reallynotforloan"; - $substatus = exists $descs{$item->notforloan} ? $descs{$item->notforloan}->{opac_description} : "Not for loan"; + $substatus = $item->{notforloan_description}; } - elsif ( exists $itemtypes->{ $item->effective_itemtype } - && $itemtypes->{ $item->effective_itemtype }->{notforloan} - && $itemtypes->{ $item->effective_itemtype }->{notforloan} == 1 ) + elsif ( defined $item->{notforloan_itemtype} && $item->{notforloan_itemtype} == 1 ) { $status = "1" =~ /^($ref_status)$/ ? "reference" @@ -411,13 +386,13 @@ sub buildKohaItemsNamespace { else { $status = "available"; } - my $homebranch = C4::Koha::xml_escape($branches{$item->homebranch}); - my $holdingbranch = C4::Koha::xml_escape($branches{$item->holdingbranch}); + my $homebranch = xml_escape( $item->{homebranch} ); + my $holdingbranch = xml_escape( $item->{holdingbranch} ); my $resultbranch = C4::Context->preference('OPACResultsLibrary') eq 'homebranch' ? $homebranch : $holdingbranch; - my $location = C4::Koha::xml_escape($item->location && exists $shelflocations->{$item->location} ? $shelflocations->{$item->location} : $item->location); - my $ccode = C4::Koha::xml_escape($item->ccode && exists $ccodes->{$item->ccode} ? $ccodes->{$item->ccode} : $item->ccode); - my $itemcallnumber = C4::Koha::xml_escape($item->itemcallnumber); - my $stocknumber = C4::Koha::xml_escape($item->stocknumber); + my $location = xml_escape( $item->{location} ); + my $ccode = xml_escape( $item->{ccode} ); + my $itemcallnumber = xml_escape($item->{itemcallnumber}); + my $stocknumber = xml_escape($item->{stocknumber}); $xml .= "" . "$homebranch" diff --git a/opac/opac-shelves.pl b/opac/opac-shelves.pl index e228b3dfb0..62cc6cfb4f 100755 --- a/opac/opac-shelves.pl +++ b/opac/opac-shelves.pl @@ -349,6 +349,14 @@ if ( $op eq 'view' ) { $art_req_itypes = Koha::CirculationRules->guess_article_requestable_itemtypes({ $patron ? ( categorycode => $patron->categorycode ) : () }); } + # Build lookup tables now to avoid fetching later + my %branches = map { $_->branchcode => $_->branchname } Koha::Libraries->search({}, { order_by => 'branchname' }); + my $shelflocations = { map { $_->{authorised_value} => $_->{opac_description} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => '', kohafield => 'items.location' } ) }; + my $notforloan_descriptions = { map { $_->{authorised_value} => $_->{opac_description} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { kohafield => 'items.notforloan' } ) }; + my $ccodes = { map { $_->{authorised_value} => $_->{opac_description} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => "", kohafield => 'items.ccode' } ) }; + my $itemtypes = Koha::ItemTypes->search_with_localization; + my %itemtypes = map { $_->{itemtype} => $_ } @{ $itemtypes->unblessed }; + my @items_info; while ( my $content = $contents->next ) { my $biblionumber = $content->biblionumber; @@ -394,11 +402,21 @@ if ( $op eq 'view' ) { my $items = $biblio->items->filter_by_visible_in_opac({ patron => $patron }); my $allow_onshelf_holds; + my @items; while ( my $item = $items->next ) { # This method must take a Koha::Items rs $allow_onshelf_holds ||= Koha::CirculationRules->get_onshelfholds_policy( { item => $item, patron => $patron } ); + my $item_hash = $item->unblessed; + $item_hash->{itemtype} = $item->effective_itemtype; + $item_hash->{homebranch} = $branches{ $item->homebranch }; + $item_hash->{holdingbranch} = $branches{ $item->holdingbranch }; + $item_hash->{notforloan_description} = $item->notforloan && exists $notforloan_descriptions->{ $item->notforloan } ? $notforloan_descriptions->{ $item->notforloan } : "Not for loan"; + $item_hash->{location} = $item->{location} && exists $shelflocations->{$item->location} ? $shelflocations->{$item->location} : $item->location ; + $item_hash->{ccode_description} = $item->{ccode} && exists $ccodes->{$item->ccode} ? $ccodes->{$item->ccode} : $item->ccode ; + $item_hash->{notforloan_itemtype} = $itemtypes{ $itemtype }->{notforloan}; + push @items, $item_hash; } diff --git a/t/db_dependent/XSLT.t b/t/db_dependent/XSLT.t index 8f08abdb80..ded3adb265 100755 --- a/t/db_dependent/XSLT.t +++ b/t/db_dependent/XSLT.t @@ -18,7 +18,7 @@ use Modern::Perl; use MARC::Record; -use Test::More tests => 4; +use Test::More tests => 3; use Test::Warn; use t::lib::TestBuilder; use t::lib::Mocks; @@ -48,190 +48,97 @@ subtest 'transformMARCXML4XSLT tests' => sub { }; subtest 'buildKohaItemsNamespace status tests' => sub { - plan tests => 17; + plan tests => 14; t::lib::Mocks::mock_preference('Reference_NFL_Statuses', '1|2'); t::lib::Mocks::mock_preference( 'OPACResultsLibrary', 'holdingbranch' ); t::lib::Mocks::mock_preference( 'OPACResultsMaxItems', '2' ); - my $itype = $builder->build_object({ class => 'Koha::ItemTypes' }); - my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' }); - my $holdinglibrary = $builder->build_object({ class => 'Koha::Libraries' }); - my $item = $builder->build_sample_item({ itype => $itype->itemtype }); - $item->holdingbranch( $holdinglibrary->branchcode )->store; - $item->biblioitem->itemtype($itemtype->itemtype)->store; + my $holdingbranch = $builder->build_object({ class => 'Koha::Libraries' }); + my $item = $builder->build_sample_item({ holdingbranch => $holdingbranch->branchcode })->unblessed; - my $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); + my $xml = C4::XSLT::buildKohaItemsNamespace([$item]); like($xml,qr{available},"Item is available when no other status applied"); # notforloan { - t::lib::Mocks::mock_preference('item-level_itypes', 0); - $item->notforloan(0)->store; - Koha::ItemTypes->find($item->itype)->notforloan(0)->store; - Koha::ItemTypes->find($item->biblioitem->itemtype)->notforloan(1)->store; - $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); - like($xml,qr{reference},"reference if positive itype notforloan value"); - - t::lib::Mocks::mock_preference('item-level_itypes', 1); - Koha::ItemTypes->find($item->itype)->notforloan(1)->store; - Koha::ItemTypes->find($item->biblioitem->itemtype)->notforloan(0)->store; - $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); - like($xml,qr{reference},"reference if positive itemtype notforloan value"); - Koha::ItemTypes->find($item->itype)->notforloan(0)->store; - - my $substatus = Koha::AuthorisedValues->search({ category => 'NOT_LOAN', authorised_value => -1 })->next->lib; - $item->notforloan(-1)->store; - $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); - like($xml,qr{reallynotforloan},"reallynotforloan if negative notforloan value"); - like($xml,qr{$substatus},"substatus set if negative notforloan value"); - - $item->notforloan(1)->store; - $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); - like($xml,qr{reference},"reference if positive notforloan value"); + + $item->{notforloan} = 1; + $xml = C4::XSLT::buildKohaItemsNamespace([$item]); + like($xml,qr{reference},"reference if itemtype notforloan value in Reference_NFL_Statuses"); + + $item->{notforloan} = -1; + $item->{notforloan_description} = 'nopenope'; + $xml = C4::XSLT::buildKohaItemsNamespace( [$item]); + like($xml,qr{reallynotforloan},"reallynotforloan if notforloan value not in Reference_NFL_Statuses"); + like($xml,qr{nopenope},"substatus set if notforloan value not in Reference_NFL_Statuses"); # But now make status notforloan==1 count under Not available + $item->{notforloan} = 1; t::lib::Mocks::mock_preference('Reference_NFL_Statuses', '2'); - $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); + $xml = C4::XSLT::buildKohaItemsNamespace( [$item]); like($xml,qr{reallynotforloan},"reallynotforloan when we change Reference_NFL_Statuses"); t::lib::Mocks::mock_preference('Reference_NFL_Statuses', '1|2'); } - $item->onloan('2001-01-01')->store; - $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); - like( $xml, qr/other<\/status>/, "Checked out is part of other statuses" ); + $item->{onloan} = '2001-01-01'; + $xml = C4::XSLT::buildKohaItemsNamespace( [$item]); like($xml,qr{Checked out},"Checked out status takes precedence over Not for loan"); - $item->withdrawn(1)->store; - $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); + $item->{withdrawn} = 1; + $xml = C4::XSLT::buildKohaItemsNamespace( [$item]); like($xml,qr{Withdrawn},"Withdrawn status takes precedence over Checked out"); - $item->itemlost(1)->store; - $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); + $item->{itemlost} = 1; + $xml = C4::XSLT::buildKohaItemsNamespace( [$item]); like($xml,qr{Lost},"Lost status takes precedence over Withdrawn"); - $item->damaged(1)->store; - $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); + $item->{damaged} = 1; + $xml = C4::XSLT::buildKohaItemsNamespace( [$item]); like($xml,qr{Damaged},"Damaged status takes precedence over Lost"); $builder->build({ source => "Branchtransfer", value => { - itemnumber => $item->itemnumber, + itemnumber => $item->{itemnumber}, datearrived => undef, datecancelled => undef, } }); - $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); + $xml = C4::XSLT::buildKohaItemsNamespace( [$item]); like($xml,qr{In transit},"In-transit status takes precedence over Damaged"); my $hold = $builder->build_object({ class => 'Koha::Holds', value => { - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber, + biblionumber => $item->{biblionumber}, + itemnumber => $item->{itemnumber}, found => 'W', priority => 0, } }); - $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); - like($xml,qr{Hold waiting},"Waiting status takes precedence over In transit (holds)"); - $hold->cancel; + $xml = C4::XSLT::buildKohaItemsNamespace( [$item]); + like($xml,qr{Waiting},"Waiting status takes precedence over In transit"); $builder->build({ source => "TmpHoldsqueue", value => { - itemnumber => $item->itemnumber + itemnumber => $item->{itemnumber} } }); - $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); + + $xml = C4::XSLT::buildKohaItemsNamespace( [$item]); like($xml,qr{Pending hold},"Pending status takes precedence over all"); - my $library_name = $holdinglibrary->branchname; - like($xml,qr{${library_name}}, "Found resultbranch / holding branch" ); + + my $resultbranch = $holdingbranch->branchcode; + like($xml,qr{${resultbranch}}, "Found resultbranch / holding branch" ); t::lib::Mocks::mock_preference('UseRecalls', 1); my $recall = $builder->build_object({ class => 'Koha::Recalls', value => { - biblio_id => $item->biblionumber, - item_id => $item->itemnumber, - pickup_library_id => $item->holdingbranch, + biblio_id => $item->{biblionumber}, + item_id => $item->{itemnumber}, + pickup_library_id => $item->{holdingbranch}, }}); $recall->set_waiting; - $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); + $xml = C4::XSLT::buildKohaItemsNamespace( [$item] ); like($xml,qr{Recall waiting},"Waiting status takes precedence over In transit (recalls)"); t::lib::Mocks::mock_preference('UseRecalls', 0); + }; $schema->storage->txn_rollback; - -subtest 'buildKohaItemsNamespace() including/omitting items tests' => sub { - - plan tests => 23; - - $schema->storage->txn_begin; - - my $biblio = $builder->build_sample_biblio; - my $biblio2 = $builder->build_sample_biblio; - - # Have two known libraries for testing purposes - my $library_1 = $builder->build_object({ class => 'Koha::Libraries' }); - my $library_2 = $builder->build_object({ class => 'Koha::Libraries' }); - my $library_3 = $builder->build_object({ class => 'Koha::Libraries' }); - - my $item_1 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber, library => $library_1->id }); - my $item_2 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber, library => $library_2->id }); - my $item_3 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber, library => $library_3->id }); - - my $items_rs = $biblio->items->search({ "me.itemnumber" => { '!=' => $item_3->itemnumber } }); - - ## Test passing items_rs only - my $xml = C4::XSLT::buildKohaItemsNamespace( $biblio->biblionumber, undef, $items_rs ); - - my $library_1_name = $library_1->branchname; - my $library_2_name = $library_2->branchname; - my $library_3_name = $library_3->branchname; - - like( $xml, qr{$library_1_name}, '$item_1 present in the XML' ); - like( $xml, qr{$library_2_name}, '$item_2 present in the XML' ); - unlike( $xml, qr{$library_3_name}, '$item_3 not present in the XML' ); - - t::lib::Mocks::mock_preference('OpacHiddenItems', 'biblionumber: ['.$biblio2->biblionumber.']'); - my $hid_rs = $biblio->items->search({ "me.itemnumber" => { '!=' => $item_3->itemnumber } })->filter_by_visible_in_opac(); - $xml = C4::XSLT::buildKohaItemsNamespace( $biblio->biblionumber, undef, $hid_rs ); - like( $xml, qr{$library_1_name}, '$item_1 present in the XML' ); - like( $xml, qr{$library_2_name}, '$item_2 present in the XML' ); - unlike( $xml, qr{$library_3_name}, '$item_3 not present in the XML' ); - - ## Test passing one item in hidden_items and items_rs - $xml = C4::XSLT::buildKohaItemsNamespace( $biblio->biblionumber, [ $item_1->itemnumber ], $items_rs->reset ); - - unlike( $xml, qr{$library_1_name}, '$item_1 not present in the XML' ); - like( $xml, qr{$library_2_name}, '$item_2 present in the XML' ); - unlike( $xml, qr{$library_3_name}, '$item_3 not present in the XML' ); - - ## Test passing both items in hidden_items and items_rs - $xml = C4::XSLT::buildKohaItemsNamespace( $biblio->biblionumber, [ $item_1->itemnumber, $item_2->itemnumber ], $items_rs->reset ); - - unlike( $xml, qr{$library_1_name}, '$item_1 not present in the XML' ); - unlike( $xml, qr{$library_2_name}, '$item_2 not present in the XML' ); - unlike( $xml, qr{$library_3_name}, '$item_3 not present in the XML' ); - is( $xml, '', 'Empty XML' ); - - ## Test passing both items in hidden_items and no items_rs - $xml = C4::XSLT::buildKohaItemsNamespace( $biblio->biblionumber, [ $item_1->itemnumber, $item_2->itemnumber, $item_3->itemnumber ] ); - - unlike( $xml, qr{$library_1_name}, '$item_1 not present in the XML' ); - unlike( $xml, qr{$library_2_name}, '$item_2 not present in the XML' ); - unlike( $xml, qr{$library_3_name}, '$item_3 not present in the XML' ); - is( $xml, '', 'Empty XML' ); - - ## Test passing one item in hidden_items and items_rs - $xml = C4::XSLT::buildKohaItemsNamespace( $biblio->biblionumber, [ $item_1->itemnumber ] ); - - unlike( $xml, qr{$library_1_name}, '$item_1 not present in the XML' ); - like( $xml, qr{$library_2_name}, '$item_2 present in the XML' ); - like( $xml, qr{$library_3_name}, '$item_3 present in the XML' ); - - ## Test not passing any param - $xml = C4::XSLT::buildKohaItemsNamespace( $biblio->biblionumber ); - - like( $xml, qr{$library_1_name}, '$item_1 present in the XML' ); - like( $xml, qr{$library_2_name}, '$item_2 present in the XML' ); - like( $xml, qr{$library_3_name}, '$item_3 present in the XML' ); - - $schema->storage->txn_rollback; -}; -- 2.30.2