From 3ab7a7bd4ff86ffb0432de30e65115b5f6d91937 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 3 Dec 2020 01:33:07 +0000 Subject: [PATCH] Bug 27138: Index host items in child records when sending to search engine On the Koha side we seperate the items from the MARC record but we do put them in the search engines For easy analytics we embed host items when displaying results after fetching from the search engine We can simpify things and improve results by including the host items when indexing To test: 1 - Enable EasyAnalyticalRecords 2 - Find a record in the staff client 3 - From details page select Edit->Link to host record 4 - Attach a barcode from a different record 5 - Search for that barcode in the staff client 6 - Only host record is returned 7 - Perform a search that returns the child record and others 8 - Note the host item is listed on the child record in the results page 9 - Repeat on OPAC 10 - Note on opac that host item is missing on results 11 - Apply patch 12 - Search for barcode 13 - Child and host record returned on both staff and OPAC 14 - Child record correctly includes host item on staff and OPAC 15 - Connect to the Z3950 responder or zebra directly yaz-client localhost:2100 yaz-client unix:/var/run/koha/kohadev/bibliosocket 16 - Search for the child record base biblios find record_keyword show 1+10 17 - Note the record includes the host item 18 - Repeat tests for Elasticsearch --- C4/Biblio.pm | 10 ++++++-- C4/Search.pm | 23 ------------------- C4/XSLT.pm | 8 ++++++- Koha/SearchEngine/Elasticsearch/Indexer.pm | 2 +- misc/migration_tools/rebuild_zebra.pl | 14 ++++++++--- t/db_dependent/Items.t | 37 +++++++++++++++++++++++++++++- 6 files changed, 63 insertions(+), 31 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 6e53842ee1..c3c41f2620 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -1197,6 +1197,7 @@ sub GetMarcBiblio { my $biblionumber = $params->{biblionumber}; my $embeditems = $params->{embed_items} || 0; + my $embedhostitems = $params->{embed_host_items} // 0; my $opac = $params->{opac} || 0; my $borcat = $params->{borcat} // q{}; @@ -1230,7 +1231,9 @@ sub GetMarcBiblio { marc_record => $record, biblionumber => $biblionumber, opac => $opac, - borcat => $borcat }) + borcat => $borcat, + embed_host_items => $embedhostitems, + }) if ($embeditems); return $record; @@ -2636,6 +2639,9 @@ sub EmbedItemsInMarcBiblio { my $dbh = C4::Context->dbh; my $sth = $dbh->prepare("SELECT itemnumber FROM items WHERE biblionumber = ?"); $sth->execute($biblionumber); + my $all_itemnumbers = $sth->fetchall_arrayref(); + @$all_itemnumbers = map { $_->[0] } @$all_itemnumbers; + push @$all_itemnumbers, C4::Items::get_hostitemnumbers_of( $biblionumber ) if $params->{embed_host_items}; my ( $itemtag, $itemsubfield ) = GetMarcFromKohaField( "items.itemnumber" ); my @item_fields; # Array holding the actual MARC data for items to be included. @@ -2647,7 +2653,7 @@ sub EmbedItemsInMarcBiblio { && ( C4::Context->preference('OpacHiddenItems') !~ /^\s*$/ ); require C4::Items; - while ( my ($itemnumber) = $sth->fetchrow_array ) { + foreach my $itemnumber ( @$all_itemnumbers ) { next if @$itemnumbers and not grep { $_ == $itemnumber } @$itemnumbers; my $item; if ( $opachiddenitems ) { diff --git a/C4/Search.pm b/C4/Search.pm index f2e73ee5a4..6cd5bd58b9 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -1846,29 +1846,6 @@ sub searchResults { my @fields = $marcrecord->field($itemtag); my $marcflavor = C4::Context->preference("marcflavour"); - # adding linked items that belong to host records - if ( C4::Context->preference('EasyAnalyticalRecords') ) { - my $analyticsfield = '773'; - if ($marcflavor eq 'MARC21' || $marcflavor eq 'NORMARC') { - $analyticsfield = '773'; - } elsif ($marcflavor eq 'UNIMARC') { - $analyticsfield = '461'; - } - foreach my $hostfield ( $marcrecord->field($analyticsfield)) { - my $hostbiblionumber = $hostfield->subfield("0"); - my $linkeditemnumber = $hostfield->subfield("9"); - if( $hostbiblionumber ) { - my $linkeditemmarc = C4::Items::GetMarcItem( $hostbiblionumber, $linkeditemnumber ); - if ($linkeditemmarc) { - my $linkeditemfield = $linkeditemmarc->field($itemtag); - if ($linkeditemfield) { - push( @fields, $linkeditemfield ); - } - } - } - } - } - # Setting item statuses for display my @available_items_loop; my @onloan_items_loop; diff --git a/C4/XSLT.pm b/C4/XSLT.pm index 1b3941c7fb..6ca3934c44 100644 --- a/C4/XSLT.pm +++ b/C4/XSLT.pm @@ -297,10 +297,16 @@ Is only used in this module currently. sub buildKohaItemsNamespace { my ($biblionumber, $hidden_items) = @_; + my @host_itemnumbers = C4::Items::get_hostitemnumbers_of( $biblionumber ); $hidden_items ||= []; my @items = Koha::Items->search( { - 'me.biblionumber' => $biblionumber, + -or => [ + 'me.biblionumber' => $biblionumber, + 'me.itemnumber' => { + -in => \@host_itemnumbers + } + ], 'me.itemnumber' => { not_in => $hidden_items } }, { prefetch => [ 'branchtransfers', 'reserves' ] } diff --git a/Koha/SearchEngine/Elasticsearch/Indexer.pm b/Koha/SearchEngine/Elasticsearch/Indexer.pm index e5f6474fb3..ebe3ce1964 100644 --- a/Koha/SearchEngine/Elasticsearch/Indexer.pm +++ b/Koha/SearchEngine/Elasticsearch/Indexer.pm @@ -324,7 +324,7 @@ sub index_records { sub _get_record { my ( $id, $server ) = @_; return $server eq 'biblioserver' - ? C4::Biblio::GetMarcBiblio({ biblionumber => $id, embed_items => 1 }) + ? C4::Biblio::GetMarcBiblio({ biblionumber => $id, embed_items => 1, embed_host_items => 1 }) : C4::AuthoritiesMarc::GetAuthority($id); } diff --git a/misc/migration_tools/rebuild_zebra.pl b/misc/migration_tools/rebuild_zebra.pl index cb42164e06..e08c8ad495 100755 --- a/misc/migration_tools/rebuild_zebra.pl +++ b/misc/migration_tools/rebuild_zebra.pl @@ -504,13 +504,21 @@ sub export_marc_records_from_sth { ? GetXmlBiblio( $record_number ) : GetAuthorityXML( $record_number ); if ($record_type eq 'biblio'){ - my @items = GetItemsInfo($record_number); + my @host_itemnumbers = C4::Items::get_hostitemnumbers_of( $record_number ); + my @items = Koha::Items->search({ + -or => [ + biblionumber => $record_number, + itemnumber => { + -in => \@host_itemnumbers + } + ] + }); if (@items){ my $record = MARC::Record->new; $record->encoding('UTF-8'); my @itemsrecord; foreach my $item (@items){ - my $record = Item2Marc($item, $record_number); + my $record = Item2Marc($item->unblessed, $record_number); push @itemsrecord, $record->field($itemtag); } $record->insert_fields_ordered(@itemsrecord); @@ -673,7 +681,7 @@ sub get_raw_marc_record { my $marc; if ($record_type eq 'biblio') { - eval { $marc = C4::Biblio::GetMarcBiblio({ biblionumber => $record_number, embed_items => 1 }); }; + eval { $marc = C4::Biblio::GetMarcBiblio({ biblionumber => $record_number, embed_items => 1, embed_host_items => 1 }); }; if ($@ || !$marc) { # here we do warn since catching an exception # means that the bib was found but failed diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t index 4f94041643..32651f4b1b 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -680,7 +680,7 @@ subtest 'Koha::Item(s) tests' => sub { }; subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { - plan tests => 8; + plan tests => 11; $schema->storage->txn_begin(); @@ -724,6 +724,12 @@ subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { push @itemnumbers, $itemnumber; } + my $host_item = $builder->build_sample_item({ homebranch => $library2->{branchcode} }); + my $child_field = PrepHostMarcField( $host_item->biblionumber, $host_item->itemnumber, 'MARC21'); + my $child_record = $biblio->metadata->record; + $child_record->append_fields($child_field); + ModBiblio( $child_record, $biblio->biblionumber, '' ); + # Emptied the OpacHiddenItems pref t::lib::Mocks::mock_preference( 'OpacHiddenItems', '' ); @@ -748,6 +754,21 @@ subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { C4::Biblio::EmbedItemsInMarcBiblio({ marc_record => $record, biblionumber => $biblio->biblionumber, + embed_host_items => 1 + }); + @items = $record->field($itemfield); + is( scalar @items, $number_of_items + 1, 'Should return all items plus host items' ); + + $marc_with_items = C4::Biblio::GetMarcBiblio({ + biblionumber => $biblio->biblionumber, + embed_items => 1, + embed_host_items => 1 + }); + is_deeply( $record, $marc_with_items, 'A direct call to GetMarcBiblio with items matches'); + + C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $record, + biblionumber => $biblio->biblionumber, item_numbers => [ $itemnumbers[1], $itemnumbers[3] ] }); @items = $record->field($itemfield); is( scalar @items, 2, 'Should return all items present in the list' ); @@ -782,6 +803,20 @@ subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { 'For OPAC, the pref OpacHiddenItems should have been take into account. Only items with homebranch ne CPL should have been embedded' ); + C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $record, + biblionumber => $biblio->biblionumber, + opac => 1, + embed_host_items => 1 + }); + @items = $record->field($itemfield); + is( + scalar @items, + $number_of_items - $number_of_items_with_homebranch_is_CPL + 1, +'For OPAC, the pref OpacHiddenItems should have been take into account and host items embedded. Only items with homebranch ne CPL should have been embedded' + ); + + $opachiddenitems = " homebranch: ['$library1->{branchcode}', '$library2->{branchcode}']"; t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems ); -- 2.11.0