From 9746276aeeec2d6ebe031060c32508f5aefc189a Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 25 Aug 2021 15:26:23 +0000 Subject: [PATCH] Bug 28371: Passpreviously fetched branches and itemtypes through and fetch all needed AV at once This patch updates the searchResuls code to pass through the pre-constructed branches and itemtype lookups to XSLTParse4Display to avoid repeating this It also updates getAuthorisedValues4MARCSubfields to fetch the values for mapped subfields and pass then through to transforMarc4XSLT Note that we currently blank invalid branches and itemtypes - I presrve this, we should open another bug if we want to change this behaviour Changes are covered by tests To test: 1 - Perform searches in OPAC and staff client that return many records 2 - Use the 'Network' tab on the browser console (opened with F12 usually) to see the time taken 3 - Note the speed before the patch 4 - Apply patch 5 - restart all the things 6 - Note improvement in speed **Note: The improvement is more drastic the more items per record, try adding large numbers of items to your search results to test** --- C4/Search.pm | 4 +++- C4/XSLT.pm | 58 ++++++++++++++++++++++++++++++++++------------------ 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/C4/Search.pm b/C4/Search.pm index 72c81ec06c..7307ce3d70 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -1970,7 +1970,9 @@ sub searchResults { ), fix_amps => 1, hidden_items => \@hiddenitems, - xslt_variables => $xslt_variables + xslt_variables => $xslt_variables, + branches => \%branches, + itemtypes => \%itemtypes } ); } diff --git a/C4/XSLT.pm b/C4/XSLT.pm index 2de14b89a5..38126096ec 100644 --- a/C4/XSLT.pm +++ b/C4/XSLT.pm @@ -65,7 +65,7 @@ Is only used in this module currently. =cut sub transformMARCXML4XSLT { - my ($biblionumber, $record) = @_; + my ($biblionumber, $record, $branches, $itemtypes, $av) = @_; my $frameworkcode = GetFrameworkCode($biblionumber) || ''; my $tagslib = &GetMarcStructure(1, $frameworkcode, { unsafe => 1 }); my @fields; @@ -75,26 +75,39 @@ sub transformMARCXML4XSLT { }; if ($@) { warn "PROBLEM WITH RECORD"; next; } my $marcflavour = C4::Context->preference('marcflavour'); - my $av = getAuthorisedValues4MARCSubfields($frameworkcode); foreach my $tag ( keys %$av ) { foreach my $field ( $record->field( $tag ) ) { - if ( $av->{ $tag } ) { + if ( defined $av->{ $tag } ) { my @new_subfields = (); + my $updated = 0; for my $subfield ( $field->subfields() ) { my ( $letter, $value ) = @$subfield; - # Replace the field value with the authorised value *except* for MARC21/NORMARC field 942$n (suppression in opac) - if ( !( $tag eq '942' && $subfield->[0] eq 'n' ) || $marcflavour eq 'UNIMARC' ) { - $value = GetAuthorisedValueDesc( $tag, $letter, $value, '', $tagslib ) - if $av->{ $tag }->{ $letter }; + if( defined $av->{ $tag }->{ $letter } ){ + my $category = $av->{$tag}->{$letter}->{category}; + # Replace the field value with the authorised value *except* for MARC21/NORMARC field 942$n (suppression in opac) + if( $category eq 'branches' ){ + $value = defined $branches->{$value} ? $branches->{$value}: q{} ; + $updated = 1; + } + elsif( $category eq 'itemtypes' ){ + $value = defined $itemtypes->{$value}->{translated_description} ? $itemtypes->{$value}->{translated_description} : q{}; + $updated = 1; + } + elsif ( !( $tag eq '942' && $subfield->[0] eq 'n' ) || $marcflavour eq 'UNIMARC' ) { + if ( defined $av->{$tag}->{$letter}->{$category}->{$value}->{lib} ) { + $value = $av->{$tag}->{$letter}->{$category}->{$value}->{lib} || $value; + $updated = 1; + } + } } push( @new_subfields, $letter, $value ); - } + } $field ->replace_with( MARC::Field->new( $tag, $field->indicator(1), $field->indicator(2), @new_subfields - ) ); + ) ) if $updated; } } } @@ -112,15 +125,17 @@ sub getAuthorisedValues4MARCSubfields { my ($frameworkcode) = @_; unless ( $authval_per_framework{ $frameworkcode } ) { my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("SELECT DISTINCT tagfield, tagsubfield + my $sth = $dbh->prepare("SELECT DISTINCT tagfield, tagsubfield, authorised_value FROM marc_subfield_structure WHERE authorised_value IS NOT NULL AND authorised_value!='' AND frameworkcode=?"); $sth->execute( $frameworkcode ); my $av = { }; - while ( my ( $tag, $letter ) = $sth->fetchrow() ) { - $av->{ $tag }->{ $letter } = 1; + while ( my ( $tag, $letter, $category ) = $sth->fetchrow() ) { + my $authorised_values = { map { $_->{authorised_value} => $_ } @{ Koha::AuthorisedValues->search({ category => $category })->unblessed } }; + $av->{ $tag }->{ $letter }->{category} = $category; + $av->{ $tag }->{ $letter }->{$category} = $authorised_values unless ( $category eq 'itemtypes' || $category eq 'branches' ); } $authval_per_framework{ $frameworkcode } = $av; } @@ -252,16 +267,22 @@ sub XSLTParse4Display { my $hidden_items = $params->{hidden_items} || []; my $variables = $params->{xslt_variables}; my $items_rs = $params->{items_rs}; + my $branches = $params->{branches}; + my $itemtypes = $params->{itemtypes}; + + $branches = { map { $_->branchcode => $_->branchname } Koha::Libraries->search({}, { order_by => 'branchname' }) } unless $branches; + $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search_with_localization->unblessed } } unless $itemtypes; my $xslfilename = get_xsl_filename( $xslsyspref); # grab the XML, run it through our stylesheet, push it out to the browser - my $record = transformMARCXML4XSLT($biblionumber, $orig_record); + my $authorised_values = getAuthorisedValues4MARCSubfields(""); + my $record = transformMARCXML4XSLT($biblionumber, $orig_record, $branches, $itemtypes, $authorised_values ); my $itemsxml; 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($biblionumber, $hidden_items, $items_rs, $branches, $itemtypes); } my $xmlrecord = $record->as_xml(C4::Context->preference('marcflavour')); @@ -319,7 +340,7 @@ Is only used in this module currently. =cut sub buildKohaItemsNamespace { - my ($biblionumber, $hidden_items, $items_rs) = @_; + my ($biblionumber, $hidden_items, $items_rs, $branches, $itemtypes) = @_; $hidden_items ||= []; @@ -339,9 +360,6 @@ sub buildKohaItemsNamespace { 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' }); - - 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'; @@ -389,8 +407,8 @@ sub buildKohaItemsNamespace { else { $status = "available"; } - my $homebranch = xml_escape($branches{$item->homebranch}); - my $holdingbranch = xml_escape($branches{$item->holdingbranch}); + my $homebranch = xml_escape($branches->{$item->homebranch}); + my $holdingbranch = xml_escape($branches->{$item->holdingbranch}); my $location = xml_escape($item->location && exists $shelflocations->{$item->location} ? $shelflocations->{$item->location} : $item->location); my $ccode = xml_escape($item->ccode && exists $ccodes->{$item->ccode} ? $ccodes->{$item->ccode} : $item->ccode); my $itemcallnumber = xml_escape($item->itemcallnumber); -- 2.20.1