From ebaf56b9c95a1f47a59a4e171d0ca234a4323222 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 | 59 ++++++++++++++++++++++++++++++++++------------------ 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/C4/Search.pm b/C4/Search.pm index f36e3364b3..860c432927 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -1981,7 +1981,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 125f887cb0..0ef192916d 100644 --- a/C4/XSLT.pm +++ b/C4/XSLT.pm @@ -62,7 +62,7 @@ Replaces codes with authorized values in a MARC::Record object =cut sub transformMARCXML4XSLT { - my ($biblionumber, $record, $opac) = @_; + my ($biblionumber, $record, $opac, $branches, $itemtypes, $av) = @_; my $frameworkcode = GetFrameworkCode($biblionumber) || ''; my $tagslib = &GetMarcStructure(1, $frameworkcode, { unsafe => 1 }); my @fields; @@ -72,26 +72,40 @@ 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 field 942$n (suppression in opac) - if ( !( $tag eq '942' && $subfield->[0] eq 'n' ) || $marcflavour eq 'UNIMARC' ) { - $value = GetAuthorisedValueDesc( $tag, $letter, $value, '', $tagslib, undef, $opac ) - 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' ) { + my $desc_field = $opac ? "lib_opac" : "lib"; + if ( defined $av->{$tag}->{$letter}->{$category}->{$value}->{$desc_field} ) { + $value = $av->{$tag}->{$letter}->{$category}->{$value}->{$desc_field} || $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; } } } @@ -109,15 +123,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; } @@ -250,16 +266,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, undef, $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 +341,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 +361,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'; @@ -396,8 +415,8 @@ 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 = C4::Koha::xml_escape($branches->{$item->homebranch}); + my $holdingbranch = C4::Koha::xml_escape($branches->{$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); -- 2.20.1