From ff098da741bc62ee05665c1ff32db99e1603367a 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 Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Michael Sutherland --- C4/Biblio.pm | 11 +++++-- C4/XSLT.pm | 6 +++- Koha/SearchEngine/Elasticsearch/Indexer.pm | 2 +- misc/migration_tools/rebuild_zebra.pl | 14 ++++++-- t/db_dependent/Items.t | 37 +++++++++++++++++++++- 5 files changed, 62 insertions(+), 8 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 69e488a1a1..e560cdfec1 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -1210,6 +1210,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{}; @@ -1243,7 +1244,9 @@ sub GetMarcBiblio { marc_record => $record, biblionumber => $biblionumber, opac => $opac, - borcat => $borcat }) + borcat => $borcat, + embed_host_items => $embedhostitems, + }) if ($embeditems); return $record; @@ -2604,6 +2607,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. @@ -2614,7 +2620,8 @@ sub EmbedItemsInMarcBiblio { my $opachiddenitems = $opac && ( C4::Context->preference('OpacHiddenItems') !~ /^\s*$/ ); - while ( my ($itemnumber) = $sth->fetchrow_array ) { + require C4::Items; + foreach my $itemnumber ( @$all_itemnumbers ) { next if @$itemnumbers and not grep { $_ == $itemnumber } @$itemnumbers; my $item; if ( $opachiddenitems ) { diff --git a/C4/XSLT.pm b/C4/XSLT.pm index 125f887cb0..ae29fc73db 100644 --- a/C4/XSLT.pm +++ b/C4/XSLT.pm @@ -321,6 +321,7 @@ Is only used in this module currently. sub buildKohaItemsNamespace { my ($biblionumber, $hidden_items, $items_rs) = @_; + my @host_itemnumbers = C4::Items::get_hostitemnumbers_of( $biblionumber ); $hidden_items ||= []; my $query = {}; @@ -328,7 +329,10 @@ sub buildKohaItemsNamespace { if $hidden_items; unless ( $items_rs && ref($items_rs) eq 'Koha::Items' ) { - $query->{'me.biblionumber'} = $biblionumber; + $query->{ '-or' } = [ + 'me.biblionumber' => $biblionumber, + 'me.itemnumber' => { -in => \@host_itemnumbers } + ]; $items_rs = Koha::Items->new; } diff --git a/Koha/SearchEngine/Elasticsearch/Indexer.pm b/Koha/SearchEngine/Elasticsearch/Indexer.pm index 0a10832d2e..8851d6ce9b 100644 --- a/Koha/SearchEngine/Elasticsearch/Indexer.pm +++ b/Koha/SearchEngine/Elasticsearch/Indexer.pm @@ -331,7 +331,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 718c30533d..77f414efea 100755 --- a/misc/migration_tools/rebuild_zebra.pl +++ b/misc/migration_tools/rebuild_zebra.pl @@ -509,13 +509,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); @@ -678,7 +686,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 120d9b9307..5f66285514 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', '' ); @@ -745,6 +751,21 @@ subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { embed_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, + 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, @@ -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.20.1