Bugzilla – Attachment 144200 Details for
Bug 27138
Host items are not included in Z39 results when using EasyAnalytics
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 27138: Index host items in child records when sending to search engine
Bug-27138-Index-host-items-in-child-records-when-s.patch (text/plain), 12.46 KB, created by
Nick Clemens (kidclamp)
on 2022-11-23 12:52:11 UTC
(
hide
)
Description:
Bug 27138: Index host items in child records when sending to search engine
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2022-11-23 12:52:11 UTC
Size:
12.46 KB
patch
obsolete
>From 9a7815f34f5c33f8d12a8ec89fd9a82bfbb75e53 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >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 <andrew@bywatersolutions.com> > >Signed-off-by: Michael Sutherland <sudrland@vt.edu> >--- > C4/XSLT.pm | 6 +- > Koha/Biblio/Metadata.pm | 11 +- > Koha/SearchEngine/Elasticsearch/Indexer.pm | 3 +- > misc/migration_tools/rebuild_zebra.pl | 14 +- > t/db_dependent/Items.t | 155 +++++++++++++++++++++ > 5 files changed, 183 insertions(+), 6 deletions(-) > >diff --git a/C4/XSLT.pm b/C4/XSLT.pm >index 21d270e3df..97898f550e 100644 >--- a/C4/XSLT.pm >+++ b/C4/XSLT.pm >@@ -265,6 +265,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 = {}; >@@ -272,7 +273,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/Biblio/Metadata.pm b/Koha/Biblio/Metadata.pm >index b6d532ea40..20017a2943 100644 >--- a/Koha/Biblio/Metadata.pm >+++ b/Koha/Biblio/Metadata.pm >@@ -21,7 +21,7 @@ use MARC::File::XML; > use Scalar::Util qw( blessed ); > > use C4::Biblio qw( GetMarcFromKohaField ); >-use C4::Items qw( GetMarcItem ); >+use C4::Items qw( GetMarcItem get_hostitemnumbers_of ); > use Koha::Database; > use Koha::Exceptions::Metadata; > >@@ -157,9 +157,14 @@ sub _embed_items { > $record->delete_field($field); > } > >- my $biblio = Koha::Biblios->find($biblionumber); >+ push @$itemnumbers, C4::Items::get_hostitemnumbers_of( $biblionumber ) if $params->{embed_host_items}; > >- my $items = $biblio->items; >+ my $items = Koha::Items->search([ >+ 'me.biblionumber' => $biblionumber, >+ 'me.itemnumber' => { >+ -in => $itemnumbers >+ } >+ ]); > if ( @$itemnumbers ) { > $items = $items->search({ itemnumber => { -in => $itemnumbers } }); > } >diff --git a/Koha/SearchEngine/Elasticsearch/Indexer.pm b/Koha/SearchEngine/Elasticsearch/Indexer.pm >index 6101a3a372..d1201e4927 100644 >--- a/Koha/SearchEngine/Elasticsearch/Indexer.pm >+++ b/Koha/SearchEngine/Elasticsearch/Indexer.pm >@@ -341,13 +341,14 @@ sub _get_record { > > if ( $self->index eq $Koha::SearchEngine::BIBLIOS_INDEX ) { > my $biblio = Koha::Biblios->find($record_id); >- $record = $biblio->metadata->record( { embed_items => 1 } ) >+ $record = $biblio->metadata->record( { embed_items => 1, embed_host_items => 1 } ) > if $biblio; > } else { > $record = C4::AuthoritiesMarc::GetAuthority($record_id); > } > > return $record; >+ > } > > =head2 delete_index($biblionums) >diff --git a/misc/migration_tools/rebuild_zebra.pl b/misc/migration_tools/rebuild_zebra.pl >index ec19ce1f5c..25df7826b5 100755 >--- a/misc/migration_tools/rebuild_zebra.pl >+++ b/misc/migration_tools/rebuild_zebra.pl >@@ -511,7 +511,15 @@ sub export_marc_records_from_sth { > if ($record_type eq 'biblio'){ > my $biblio = Koha::Biblios->find($record_number); > next unless $biblio; >- my $items = $biblio->items; >+ 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->count){ > my $record = MARC::Record->new; > $record->encoding('UTF-8'); >@@ -680,10 +688,14 @@ sub get_raw_marc_record { > > my $marc; > if ($record_type eq 'biblio') { >+<<<<<<< HEAD > eval { > my $biblio = Koha::Biblios->find($record_number); > $marc = $biblio->metadata->record({ embed_items => 1 }); > }; >+======= >+ eval { $marc = C4::Biblio::GetMarcBiblio({ biblionumber => $record_number, embed_items => 1, embed_host_items => 1 }); }; >+>>>>>>> Bug 27138: Index host items in child records when sending to search engine > 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 3f8bb8fd62..b4fa518a13 100755 >--- a/t/db_dependent/Items.t >+++ b/t/db_dependent/Items.t >@@ -585,6 +585,161 @@ subtest 'Koha::Item(s) tests' => sub { > $schema->storage->txn_rollback; > }; > >+subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { >+ plan tests => 11; >+ >+ $schema->storage->txn_begin(); >+ >+ my $builder = t::lib::TestBuilder->new; >+ my $library1 = $builder->build({ >+ source => 'Branch', >+ }); >+ my $library2 = $builder->build({ >+ source => 'Branch', >+ }); >+ my $itemtype = $builder->build({ >+ source => 'Itemtype', >+ }); >+ >+ my $biblio = $builder->build_sample_biblio(); >+ my $item_infos = [ >+ { homebranch => $library1->{branchcode}, holdingbranch => $library1->{branchcode} }, >+ { homebranch => $library1->{branchcode}, holdingbranch => $library1->{branchcode} }, >+ { homebranch => $library1->{branchcode}, holdingbranch => $library1->{branchcode} }, >+ { homebranch => $library2->{branchcode}, holdingbranch => $library2->{branchcode} }, >+ { homebranch => $library2->{branchcode}, holdingbranch => $library2->{branchcode} }, >+ { homebranch => $library1->{branchcode}, holdingbranch => $library2->{branchcode} }, >+ { homebranch => $library1->{branchcode}, holdingbranch => $library2->{branchcode} }, >+ { homebranch => $library1->{branchcode}, holdingbranch => $library2->{branchcode} }, >+ ]; >+ my $number_of_items = scalar @$item_infos; >+ my $number_of_items_with_homebranch_is_CPL = >+ grep { $_->{homebranch} eq $library1->{branchcode} } @$item_infos; >+ >+ my @itemnumbers; >+ for my $item_info (@$item_infos) { >+ my $itemnumber = $builder->build_sample_item( >+ { >+ biblionumber => $biblio->biblionumber, >+ homebranch => $item_info->{homebranch}, >+ holdingbranch => $item_info->{holdingbranch}, >+ itype => $itemtype->{itemtype} >+ } >+ )->itemnumber; >+ >+ 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', '' ); >+ >+ my ($itemfield) = >+ C4::Biblio::GetMarcFromKohaField( 'items.itemnumber' ); >+ my $record = C4::Biblio::GetMarcBiblio({ biblionumber => $biblio->biblionumber }); >+ warning_is { C4::Biblio::EmbedItemsInMarcBiblio() } >+ { carped => 'EmbedItemsInMarcBiblio: No MARC record passed' }, >+ 'Should carp is no record passed.'; >+ >+ C4::Biblio::EmbedItemsInMarcBiblio({ >+ marc_record => $record, >+ biblionumber => $biblio->biblionumber }); >+ my @items = $record->field($itemfield); >+ is( scalar @items, $number_of_items, 'Should return all items' ); >+ >+ my $marc_with_items = C4::Biblio::GetMarcBiblio({ >+ biblionumber => $biblio->biblionumber, >+ 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, >+ item_numbers => [ $itemnumbers[1], $itemnumbers[3] ] }); >+ @items = $record->field($itemfield); >+ is( scalar @items, 2, 'Should return all items present in the list' ); >+ >+ C4::Biblio::EmbedItemsInMarcBiblio({ >+ marc_record => $record, >+ biblionumber => $biblio->biblionumber, >+ opac => 1 }); >+ @items = $record->field($itemfield); >+ is( scalar @items, $number_of_items, 'Should return all items for opac' ); >+ >+ my $opachiddenitems = " >+ homebranch: ['$library1->{branchcode}']"; >+ t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems ); >+ >+ C4::Biblio::EmbedItemsInMarcBiblio({ >+ marc_record => $record, >+ biblionumber => $biblio->biblionumber }); >+ @items = $record->field($itemfield); >+ is( scalar @items, >+ $number_of_items, >+ 'Even with OpacHiddenItems set, all items should have been embedded' ); >+ >+ C4::Biblio::EmbedItemsInMarcBiblio({ >+ marc_record => $record, >+ biblionumber => $biblio->biblionumber, >+ opac => 1 }); >+ @items = $record->field($itemfield); >+ is( >+ scalar @items, >+ $number_of_items - $number_of_items_with_homebranch_is_CPL, >+'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 ); >+ C4::Biblio::EmbedItemsInMarcBiblio({ >+ marc_record => $record, >+ biblionumber => $biblio->biblionumber, >+ opac => 1 }); >+ @items = $record->field($itemfield); >+ is( >+ scalar @items, >+ 0, >+'For OPAC, If all items are hidden, no item should have been embedded' >+ ); >+ >+ $schema->storage->txn_rollback; >+}; >+ > subtest 'get_hostitemnumbers_of' => sub { > plan tests => 3; > >-- >2.30.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 27138
:
114126
|
115158
|
117333
|
117334
|
117346
|
117347
|
122035
|
125698
|
125699
|
125700
|
125701
|
128175
|
128176
|
128177
|
128178
|
128179
|
139401
|
139402
|
139403
|
139404
| 144200 |
144201
|
144202
|
144203