From 9380134a026c3927373557d9bb4cb6d7082a8b7c Mon Sep 17 00:00:00 2001 From: Alex Arnaud Date: Mon, 23 Apr 2018 10:25:14 +0000 Subject: [PATCH] Bug 18316: (follow-up) Koha::SearchField::search_marc_maps return a result set - code refactoring for gettings weighted fields - Koha::SearchFields::weighted_fields return a result set MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Séverine QUEUNE Rebased-by: Alex Arnaud Signed-off-by: Ere Maijala Signed-off-by: Martin Renvoize Signed-off-by: Nick Clemens --- .../Elasticsearch/QueryBuilder.pm | 8 +-- Koha/SearchField.pm | 41 +++++++------- Koha/SearchFields.pm | 10 +--- catalogue/search.pl | 6 +- .../SearchEngine/Elasticsearch/QueryBuilder.t | 55 ++++++++++++++++++- t/db_dependent/Koha/SearchField.t | 31 +++++------ 6 files changed, 97 insertions(+), 54 deletions(-) diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm index 91b09ab893..baeb6600ad 100644 --- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm +++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm @@ -230,13 +230,9 @@ sub build_query_compat { join( ' ', $self->_create_query_string(@search_params) ) || (), $self->_join_queries( $self->_convert_index_strings(@$limits) ) || () ); - my @weights = $params->{weight}; - my @w_fields = $params->{w_fields}; my @fields = '_all'; - if ( defined $weights[0] ) { - for (my $i = 0 ; $i < (scalar @weights) ; $i++ ){ - push @fields, "$w_fields[$i]^$weights[$i]"; - } + if ( defined($params->{weighted_fields}) && $params->{weighted_fields} ) { + push @fields, sprintf("%s^%s", $_->name, $_->weight) for Koha::SearchFields->weighted_fields; } # If there's no query on the left, let's remove the junk left behind diff --git a/Koha/SearchField.pm b/Koha/SearchField.pm index a734204cba..efabb8db7d 100644 --- a/Koha/SearchField.pm +++ b/Koha/SearchField.pm @@ -49,22 +49,25 @@ sub search_marc_maps { my ( $self ) = @_; my $marc_type = lc C4::Context->preference('marcflavour'); - my @marc_maps = (); my $schema = Koha::Database->new->schema; - my @marc_map_fields = $schema->resultset('SearchMarcToField')->search({ - search_field_id => $self->id - }); - - return @marc_maps unless @marc_map_fields; - - foreach my $marc_field ( @marc_map_fields ) { - my $marc_map = Koha::SearchMarcMaps->find( $marc_field->search_marc_map_id ); - push @marc_maps, $marc_map if $marc_map->marc_type eq $marc_type; - } - - return @marc_maps; - + my $marc_map_fields = $schema->resultset('SearchMarcToField')->search( + { + 'me.search_field_id' => $self->id, + 'search_marc_map.marc_type' => $marc_type + }, + { + select => [ + 'search_marc_map.index_name', + 'search_marc_map.marc_type', + 'search_marc_map.marc_field' + ], + as => [ 'index_name', 'marc_type', 'marc_field' ], + join => 'search_marc_map' + } + ); + + return $marc_map_fields; } =head3 is_mapped_biblios @@ -76,11 +79,11 @@ my $is_mapped_biblios = $search_field->is_mapped_biblios sub is_mapped_biblios { my ( $self ) = @_; - foreach my $marc_map ( $self->search_marc_maps ) { - return 1 if $marc_map->index_name eq 'biblios'; - } - - return 0; + return $self->search_marc_maps->search( + { + index_name => 'biblios' + } + )->count ? 1 : 0; } =head3 type diff --git a/Koha/SearchFields.pm b/Koha/SearchFields.pm index 3cfcec3c09..b2c985f526 100644 --- a/Koha/SearchFields.pm +++ b/Koha/SearchFields.pm @@ -44,18 +44,10 @@ my (@w_fields, @weight) = Koha::SearchFields->weighted_fields(); sub weighted_fields { my ($self) = @_; - my ($w_fields, $weight) = ([], []); - my $fields = $self->search( + return $self->search( { weight => { '>' => 0, '!=' => undef } }, { order_by => { -desc => 'weight' } } ); - - while ( my $field = $fields->next ) { - push @$w_fields, $field->name; - push @$weight, $field->weight; - } - - return ($w_fields, $weight); } =head3 type diff --git a/catalogue/search.pl b/catalogue/search.pl index d24f424af4..8cc1e81390 100755 --- a/catalogue/search.pl +++ b/catalogue/search.pl @@ -456,9 +456,9 @@ my $expanded_facet = $params->{'expand'}; # Define some global variables my ( $error,$query,$simple_query,$query_cgi,$query_desc,$limit,$limit_cgi,$limit_desc,$query_type); -my ($w_fields, $weight); +my $build_params; unless ( $cgi->param('advsearch') ) { - ($w_fields, $weight) = Koha::SearchFields->weighted_fields(); + $build_params->{weighted_fields} = 1; } my $builder = Koha::SearchEngine::QueryBuilder->new( @@ -473,7 +473,7 @@ my $searcher = Koha::SearchEngine::Search->new( $query_type ) = $builder->build_query_compat( \@operators, \@operands, \@indexes, \@limits, - \@sort_by, $scan, $lang, { w_fields => @$w_fields, weight => @$weight } ); + \@sort_by, $scan, $lang, $build_params ); ## parse the query_cgi string and put it into a form suitable for s my @query_inputs; diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t index 0a8f4cf57d..a526604552 100644 --- a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t +++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t @@ -18,10 +18,15 @@ use Modern::Perl; use Test::More tests => 2; +use C4::Context; use Test::Exception; +use Test::More tests => 3; use Koha::Database; -use Koha::SearchEngine::Elasticsearch::QueryBuilder +use Koha::SearchEngine::Elasticsearch::QueryBuilder; + +my $schema = Koha::Database->new->schema; +$schema->storage->txn_begin; subtest 'build_authorities_query_compat() tests' => sub { plan tests => 37; @@ -115,3 +120,51 @@ subtest 'build query from form subtests' => sub { }; + +subtest 'build_query with weighted fields tests' => sub { + plan tests => 4; + + my $builder = Koha::SearchEngine::Elasticsearch::QueryBuilder->new( { index => 'mydb' } ); + my $db_builder = t::lib::TestBuilder->new(); + + Koha::SearchFields->search({})->delete; + + $db_builder->build({ + source => 'SearchField', + value => { + name => 'acqdate', + label => 'acqdate', + weight => undef + } + }); + + $db_builder->build({ + source => 'SearchField', + value => { + name => 'title', + label => 'title', + weight => 25 + } + }); + + $db_builder->build({ + source => 'SearchField', + value => { + name => 'subject', + label => 'subject', + weight => 15 + } + }); + + my ( undef, $query ) = $builder->build_query_compat( undef, ['title:"donald duck"'], undef, undef, + undef, undef, undef, { weighted_fields => 1 }); + + my $fields = $query->{query}{query_string}{fields}; + + is(scalar(@$fields), 3, 'Search is done on 3 fields'); + is($fields->[0], '_all', 'First search field is _all'); + is($fields->[1], 'title^25', 'Second search field is title'); + is($fields->[2], 'subject^15', 'Third search field is subject'); +}; + +$schema->storage->txn_rollback; diff --git a/t/db_dependent/Koha/SearchField.t b/t/db_dependent/Koha/SearchField.t index 3338b594fa..737d78b112 100644 --- a/t/db_dependent/Koha/SearchField.t +++ b/t/db_dependent/Koha/SearchField.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 16; +use Test::More tests => 15; use Koha::Database; use Koha::SearchFields; @@ -73,11 +73,12 @@ my $smtf2 = $builder->build({ }); my $search_field = Koha::SearchFields->find($sf->{id}); -my @marc_map = $search_field->search_marc_maps; -is(scalar(@marc_map), 1, 'search_marc_maps should return 1 marc map'); -is($marc_map[0]->index_name, 'biblios', 'Marc map index name is biblios'); -is($marc_map[0]->marc_type, 'marc21', 'Marc map type is marc21'); -is($marc_map[0]->marc_field, '410abcdef', 'Marc map field is 410abcdef'); +my $marc_maps = $search_field->search_marc_maps; +my $marc_map = $marc_maps->next; +is($marc_maps->count, 1, 'search_marc_maps should return 1 marc map'); +is($marc_map->get_column('index_name'), 'biblios', 'Marc map index name is biblios'); +is($marc_map->get_column('marc_type'), 'marc21', 'Marc map type is marc21'); +is($marc_map->get_column('marc_field'), '410abcdef', 'Marc map field is 410abcdef'); ok($search_field->is_mapped_biblios, 'Search field 1 is mapped with biblios'); @@ -188,16 +189,14 @@ $builder->build({ } }); -my ($w_fields, $weight) = Koha::SearchFields->weighted_fields(); -is(scalar(@$w_fields), 3, 'weighted_fields should return 3 weighted fields.'); -is(scalar(@$weight), 3, 'weighted_fields should return 3 weight.'); +my @w_fields = Koha::SearchFields->weighted_fields(); +is(scalar(@w_fields), 3, 'weighted_fields should return 3 weighted fields.'); -is($w_fields->[0], 'title', 'First field is title.'); -is($w_fields->[1], 'subject', 'Second field is subject.'); -is($w_fields->[2], 'author', 'Third field is author.'); - -is($weight->[0], 25, 'Title weight is 25.'); -is($weight->[1], 15, 'Subject weight is 15.'); -is($weight->[2], 5, 'Author weight is 5.'); +is($w_fields[0]->name, 'title', 'First field is title.'); +is($w_fields[0]->weight, 25, 'Title weight is 25.'); +is($w_fields[1]->name, 'subject', 'Second field is subject.'); +is($w_fields[1]->weight, 15, 'Subject weight is 15.'); +is($w_fields[2]->name, 'author', 'Third field is author.'); +is($w_fields[2]->weight, 5, 'Author weight is 5.'); $schema->storage->txn_rollback; \ No newline at end of file -- 2.39.5