Bugzilla – Attachment 138459 Details for
Bug 28702
Improve performance of C4/XSLT/buildKohaItemsNamespace
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28702: Make buildKohaItemsNamespace take an array
Bug-28702-Make-buildKohaItemsNamespace-take-an-arr.patch (text/plain), 31.73 KB, created by
Nick Clemens (kidclamp)
on 2022-08-01 12:18:03 UTC
(
hide
)
Description:
Bug 28702: Make buildKohaItemsNamespace take an array
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2022-08-01 12:18:03 UTC
Size:
31.73 KB
patch
obsolete
>From 970445fdad120d46101e0234a852f2ed56a10249 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >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 .= > "<item>" > . "<homebranch>$homebranch</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{<status>available</status>},"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{<status>reference</status>},"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{<status>reference</status>},"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{<status>reallynotforloan</status>},"reallynotforloan if negative notforloan value"); >- like($xml,qr{<substatus>$substatus</substatus>},"substatus set if negative notforloan value"); >- >- $item->notforloan(1)->store; >- $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); >- like($xml,qr{<status>reference</status>},"reference if positive notforloan value"); >+ >+ $item->{notforloan} = 1; >+ $xml = C4::XSLT::buildKohaItemsNamespace([$item]); >+ like($xml,qr{<status>reference</status>},"reference if itemtype notforloan value in Reference_NFL_Statuses"); >+ >+ $item->{notforloan} = -1; >+ $item->{notforloan_description} = 'nopenope'; >+ $xml = C4::XSLT::buildKohaItemsNamespace( [$item]); >+ like($xml,qr{<status>reallynotforloan</status>},"reallynotforloan if notforloan value not in Reference_NFL_Statuses"); >+ like($xml,qr{<substatus>nopenope</substatus>},"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{<status>reallynotforloan</status>},"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/<status>other<\/status>/, "Checked out is part of other statuses" ); >+ $item->{onloan} = '2001-01-01'; >+ $xml = C4::XSLT::buildKohaItemsNamespace( [$item]); > like($xml,qr{<substatus>Checked out</substatus>},"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{<substatus>Withdrawn</substatus>},"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{<substatus>Lost</substatus>},"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{<substatus>Damaged</substatus>},"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{<substatus>In transit</substatus>},"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{<substatus>Hold waiting</substatus>},"Waiting status takes precedence over In transit (holds)"); >- $hold->cancel; >+ $xml = C4::XSLT::buildKohaItemsNamespace( [$item]); >+ like($xml,qr{<substatus>Waiting</substatus>},"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{<substatus>Pending hold</substatus>},"Pending status takes precedence over all"); >- my $library_name = $holdinglibrary->branchname; >- like($xml,qr{<resultbranch>${library_name}</resultbranch>}, "Found resultbranch / holding branch" ); >+ >+ my $resultbranch = $holdingbranch->branchcode; >+ like($xml,qr{<resultbranch>${resultbranch}</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{<substatus>Recall waiting</substatus>},"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{<homebranch>$library_1_name</homebranch>}, '$item_1 present in the XML' ); >- like( $xml, qr{<homebranch>$library_2_name</homebranch>}, '$item_2 present in the XML' ); >- unlike( $xml, qr{<homebranch>$library_3_name</homebranch>}, '$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{<homebranch>$library_1_name</homebranch>}, '$item_1 present in the XML' ); >- like( $xml, qr{<homebranch>$library_2_name</homebranch>}, '$item_2 present in the XML' ); >- unlike( $xml, qr{<homebranch>$library_3_name</homebranch>}, '$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{<homebranch>$library_1_name</homebranch>}, '$item_1 not present in the XML' ); >- like( $xml, qr{<homebranch>$library_2_name</homebranch>}, '$item_2 present in the XML' ); >- unlike( $xml, qr{<homebranch>$library_3_name</homebranch>}, '$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{<homebranch>$library_1_name</homebranch>}, '$item_1 not present in the XML' ); >- unlike( $xml, qr{<homebranch>$library_2_name</homebranch>}, '$item_2 not present in the XML' ); >- unlike( $xml, qr{<homebranch>$library_3_name</homebranch>}, '$item_3 not present in the XML' ); >- is( $xml, '<items xmlns="http://www.koha-community.org/items"></items>', '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{<homebranch>$library_1_name</homebranch>}, '$item_1 not present in the XML' ); >- unlike( $xml, qr{<homebranch>$library_2_name</homebranch>}, '$item_2 not present in the XML' ); >- unlike( $xml, qr{<homebranch>$library_3_name</homebranch>}, '$item_3 not present in the XML' ); >- is( $xml, '<items xmlns="http://www.koha-community.org/items"></items>', 'Empty XML' ); >- >- ## Test passing one item in hidden_items and items_rs >- $xml = C4::XSLT::buildKohaItemsNamespace( $biblio->biblionumber, [ $item_1->itemnumber ] ); >- >- unlike( $xml, qr{<homebranch>$library_1_name</homebranch>}, '$item_1 not present in the XML' ); >- like( $xml, qr{<homebranch>$library_2_name</homebranch>}, '$item_2 present in the XML' ); >- like( $xml, qr{<homebranch>$library_3_name</homebranch>}, '$item_3 present in the XML' ); >- >- ## Test not passing any param >- $xml = C4::XSLT::buildKohaItemsNamespace( $biblio->biblionumber ); >- >- like( $xml, qr{<homebranch>$library_1_name</homebranch>}, '$item_1 present in the XML' ); >- like( $xml, qr{<homebranch>$library_2_name</homebranch>}, '$item_2 present in the XML' ); >- like( $xml, qr{<homebranch>$library_3_name</homebranch>}, '$item_3 present in the XML' ); >- >- $schema->storage->txn_rollback; >-}; >-- >2.30.2
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=138459">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 28702
:
122785
|
122786
|
137939
|
137940
| 138459 |
138468