Skip to content

Commit

Permalink
Merge pull request #723 from trade-tariff/HOTT-1865-backend-tweaks-to…
Browse files Browse the repository at this point in the history
…-search

HOTT-1865: Fix several issues with search
  • Loading branch information
willfish authored Aug 5, 2022
2 parents f37189c + 15cb94b commit 13e022d
Show file tree
Hide file tree
Showing 14 changed files with 191 additions and 22 deletions.
3 changes: 2 additions & 1 deletion .env.development
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
BETA_SEARCH_MAX_HITS=10
BETA_SEARCH_DEBUG=true
BETA_SEARCH_MAX_HITS=5000
DEFAULT_API_VERSION=2
EXCHANGE_RATE_ACCESS_KEY=flibble
FRONTEND_HOST=http://localhost:3001
Expand Down
1 change: 1 addition & 0 deletions .env.test
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ AWS_ACCESS_KEY_ID=xxxxxxxxxxxxxxxxxxxx
AWS_BUCKET_NAME=trade-tariff-backend
AWS_REGION=us-east-1
AWS_SECRET_ACCESS_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
BETA_SEARCH_DEBUG=false
BETA_SEARCH_MAX_HITS=10
DEFAULT_API_VERSION=2
EXCHANGE_RATE_ACCESS_KEY=flibble
Expand Down
10 changes: 9 additions & 1 deletion app/controllers/api/beta/search_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def index
def serialized_result
Api::Beta::SearchResultSerializer.new(
search_result,
include: DEFAULT_INCLUDES,
include: includes,
meta:,
).serializable_hash
end
Expand Down Expand Up @@ -66,6 +66,14 @@ def frontend_url_for(short_code)
def all_filters
TradeTariffBackend.search_facet_classifier_configuration.all_filters
end

def includes
if TradeTariffBackend.beta_search_debug?
[:goods_nomenclature_query].concat(DEFAULT_INCLUDES)
else
DEFAULT_INCLUDES
end
end
end
end
end
4 changes: 4 additions & 0 deletions app/lib/trade_tariff_backend.rb
Original file line number Diff line number Diff line change
Expand Up @@ -256,5 +256,9 @@ def frontend_host
def beta_search_max_hits
ENV['BETA_SEARCH_MAX_HITS']
end

def beta_search_debug?
ENV['BETA_SEARCH_DEBUG'] == 'true'
end
end
end
2 changes: 2 additions & 0 deletions app/models/beta/search/facet_filter_statistic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ def build(statistic)
classifications = statistic['classifications']
.values
.map(&Beta::Search::FacetFilterClassificationStatistic.method(:build))
.sort_by(&:count)
.reverse

facet_filter_statistic = new

Expand Down
14 changes: 9 additions & 5 deletions app/models/beta/search/goods_nomenclature_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ class GoodsNomenclatureQuery
attr_accessor :original_search_query
alias_method :short_code, :original_search_query

include ContentAddressableId

content_addressable_fields :query

MULTI_MATCH_FIELDS = [
'search_references^12',
'ancestor_1_description_indexed^10', # chapter
Expand Down Expand Up @@ -47,11 +51,11 @@ def self.build(search_query_parser_result, filters = {})
end

def query
if numeric?
goods_nomenclature_item_id_term_query
else
multi_match_query
end
@query ||= if numeric?
goods_nomenclature_item_id_term_query
else
multi_match_query
end
end

def goods_nomenclature_item_id
Expand Down
24 changes: 17 additions & 7 deletions app/models/beta/search/open_search_result.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
module Beta
module Search
class OpenSearchResult
delegate :id, to: :chapter_statistics, prefix: true, allow_nil: true
delegate :id, to: :heading_statistics, prefix: true, allow_nil: true
delegate :id, to: :search_query_parser_result, prefix: true, allow_nil: true
delegate :id, to: :goods_nomenclature_query, prefix: true, allow_nil: true
delegate :id, to: :guide, prefix: true, allow_nil: true
delegate :id, to: :intercept_message, prefix: true, allow_nil: true

Expand Down Expand Up @@ -36,14 +35,25 @@ def build_hit(hit_result)
hit = Hashie::TariffMash.new

hit.score = hit_result._score
hit.goods_nomenclature_class = ActiveSupport::StringInquirer.new(hit_result._source.goods_nomenclature_class)
hit.goods_nomenclature_class = hit_result._source.goods_nomenclature_class
hit.id = hit_result._source.id
hit.goods_nomenclature_item_id = hit_result._source.goods_nomenclature_item_id
hit.producline_suffix = hit_result._source.producline_suffix
hit.description = hit_result._source.description
hit.description_indexed = hit_result._source.description_indexed
hit.chapter_description = hit_result._source.ancestor_1_description_indexed
hit.heading_description = hit_result._source.ancestor_2_description_indexed

case hit.goods_nomenclature_class
when 'Chapter'
hit.chapter_description = hit_result._source.description_indexed
hit.heading_description = nil
when 'Heading'
hit.chapter_description = hit_result._source.ancestor_1_description_indexed
hit.heading_description = hit_result._source.description_indexed
else
hit.chapter_description = hit_result._source.ancestor_1_description_indexed
hit.heading_description = hit_result._source.ancestor_2_description_indexed
end

hit.search_references = hit_result._source.search_references
hit.validity_start_date = hit_result._source.validity_start_date
hit.validity_end_date = hit_result._source.validity_end_date
Expand Down Expand Up @@ -80,15 +90,15 @@ def total_results
end

def chapter_statistics
@chapter_statistics&.values || []
(@chapter_statistics&.values || []).sort_by(&:score).reverse
end

def chapter_statistic_ids
chapter_statistics.pluck(:id)
end

def heading_statistics
@heading_statistics&.values || []
(@heading_statistics&.values || []).sort_by(&:cnt).reverse
end

def heading_statistic_ids
Expand Down
10 changes: 10 additions & 0 deletions app/serializers/api/beta/goods_nomenclature_query_serializer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module Api
module Beta
class GoodsNomenclatureQuerySerializer
include JSONAPI::Serializer
set_type :goods_nomenclature_query

attributes :query
end
end
end
4 changes: 4 additions & 0 deletions app/serializers/api/beta/search_result_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ class SearchResultSerializer
has_one :search_query_parser_result, serializer: Api::Beta::SearchQueryParserResultSerializer
has_one :intercept_message, serializer: Api::Beta::InterceptMessageSerializer

if TradeTariffBackend.beta_search_debug?
has_one :goods_nomenclature_query, serializer: Api::Beta::GoodsNomenclatureQuerySerializer
end

has_many :hits, serializer: proc { |record, _params|
if record && record.respond_to?(:goods_nomenclature_class)
"Api::Beta::#{record.goods_nomenclature_class}Serializer".constantize
Expand Down
4 changes: 2 additions & 2 deletions app/services/api/beta/search_result_statistics_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def generate_heading_statistics

def goods_nomenclature_hits_with_headings
goods_nomenclature_hits.reject do |hit|
hit.goods_nomenclature_class.chapter?
hit.goods_nomenclature_class == 'Chapter'
end
end

Expand All @@ -61,7 +61,7 @@ def mean_average_chapter_score_for(chapter_id)
def mean_average_scores
scores = []

goods_nomenclature_hits_with_headings.each do |goods_nomenclature_hit|
goods_nomenclature_hits.each do |goods_nomenclature_hit|
if yield goods_nomenclature_hit
scores << goods_nomenclature_hit.score
end
Expand Down
6 changes: 6 additions & 0 deletions spec/models/beta/search/goods_nomenclature_query_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
it { is_expected.to be_a(described_class) }
end

describe '#id' do
subject(:id) { build(:goods_nomenclature_query, :full_query).id }

it { is_expected.to eq('cbbc8118105b8e1c3cb4bb74a978fa83') }
end

describe '#query' do
let(:expected_multi_match_fields) do
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@
{
id: '2ecbb6c19ee6282b0c79dda2aeaf0192',
type: :facet_filter_statistic,
attributes: { facet_filter: 'filter_material', facet_count: 4, display_name: 'Material', question: 'Pick one of Material' },
attributes: {
facet_filter: 'filter_material',
facet_count: 4,
display_name: 'Material',
question: 'Pick one of Material',
},
relationships: {
facet_classification_statistics: {
data: [
Expand All @@ -30,7 +35,9 @@
},
relationships: {
facet_classification_statistics: {
data: [{ id: '0f9f895e9a6bb156694dd9c8bca33545', type: :facet_classification_statistic }],
data: [
{ id: '0f9f895e9a6bb156694dd9c8bca33545', type: :facet_classification_statistic },
],
},
},
},
Expand All @@ -46,8 +53,8 @@
relationships: {
facet_classification_statistics: {
data: [
{ id: '52fe3def5c7edf22d27bfa0b2fc63208', type: :facet_classification_statistic },
{ id: 'e1ff51f7d55ca6f6bbfb508996d174b1', type: :facet_classification_statistic },
{ id: '52fe3def5c7edf22d27bfa0b2fc63208', type: :facet_classification_statistic },
],
},
},
Expand Down
112 changes: 112 additions & 0 deletions spec/serializers/api/beta/goods_nomenclature_query_serializer_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
RSpec.describe Api::Beta::GoodsNomenclatureQuerySerializer do
describe '#serializable_hash' do
subject(:serializable_hash) { described_class.new(serializable).serializable_hash }

let(:serializable) { build(:goods_nomenclature_query, :full_query) }

let(:expected) do
{
data: {
id: 'cbbc8118105b8e1c3cb4bb74a978fa83',
type: :goods_nomenclature_query,
attributes: {
query: {
size: '10',
query: {
bool: {
must: [
{
multi_match: {
query: 'man',
fuzziness: 0.1,
prefix_length: 2,
tie_breaker: 0.3,
type: 'best_fields',
fields: [
'search_references^12',
'ancestor_1_description_indexed^10',
'ancestor_2_description_indexed^8',
'description_indexed^6',
'ancestor_3_description_indexed^4',
'ancestor_4_description_indexed^4',
'ancestor_5_description_indexed^4',
'ancestor_6_description_indexed^4',
'ancestor_7_description_indexed^4',
'ancestor_8_description_indexed^4',
'ancestor_9_description_indexed^4',
'ancestor_10_description_indexed^4',
'ancestor_11_description_indexed^4',
'ancestor_12_description_indexed^4',
'ancestor_13_description_indexed^4',
'goods_nomenclature_item_id',
],
},
},
],
should: [
{
multi_match: {
query: 'run',
fuzziness: 0.1,
prefix_length: 2,
tie_breaker: 0.3,
type: 'best_fields',
fields: [
'search_references^12',
'ancestor_1_description_indexed^10',
'ancestor_2_description_indexed^8',
'description_indexed^6',
'ancestor_3_description_indexed^4',
'ancestor_4_description_indexed^4',
'ancestor_5_description_indexed^4',
'ancestor_6_description_indexed^4',
'ancestor_7_description_indexed^4',
'ancestor_8_description_indexed^4',
'ancestor_9_description_indexed^4',
'ancestor_10_description_indexed^4',
'ancestor_11_description_indexed^4',
'ancestor_12_description_indexed^4',
'ancestor_13_description_indexed^4',
'goods_nomenclature_item_id',
],
},
},
{
multi_match: {
query: 'tall',
fuzziness: 0.1,
prefix_length: 2,
tie_breaker: 0.3,
type: 'best_fields',
fields: [
'search_references^12',
'ancestor_1_description_indexed^10',
'ancestor_2_description_indexed^8',
'description_indexed^6',
'ancestor_3_description_indexed^4',
'ancestor_4_description_indexed^4',
'ancestor_5_description_indexed^4',
'ancestor_6_description_indexed^4',
'ancestor_7_description_indexed^4',
'ancestor_8_description_indexed^4',
'ancestor_9_description_indexed^4',
'ancestor_10_description_indexed^4',
'ancestor_11_description_indexed^4',
'ancestor_12_description_indexed^4',
'ancestor_13_description_indexed^4',
'goods_nomenclature_item_id',
],
},
},
],
},
},
},
},
},
}
end

it { is_expected.to eq(expected) }
end
end
6 changes: 3 additions & 3 deletions spec/serializers/api/beta/search_result_serializer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,16 @@
},
heading_statistics: {
data: [
{ id: '6307', type: :heading_statistic },
{ id: '6217', type: :heading_statistic },
{ id: '6211', type: :heading_statistic },
{ id: '6209', type: :heading_statistic },
{ id: '6211', type: :heading_statistic },
{ id: '6307', type: :heading_statistic },
],
},
chapter_statistics: {
data: [
{ id: '63', type: :chapter_statistic },
{ id: '62', type: :chapter_statistic },
{ id: '63', type: :chapter_statistic },
],
},
guide: {
Expand Down

0 comments on commit 13e022d

Please sign in to comment.