diff --git a/ee/lib/elastic/latest/note_class_proxy.rb b/ee/lib/elastic/latest/note_class_proxy.rb index 70bf1fb44cc211538c7e5a3f884c82d3b24a2bba..7326eb3d58a4d17c9186dfed09007ad59b27136c 100644 --- a/ee/lib/elastic/latest/note_class_proxy.rb +++ b/ee/lib/elastic/latest/note_class_proxy.rb @@ -31,8 +31,6 @@ def elastic_search(query, options: {}) query_hash = ::Search::Elastic::Filters.by_noteable_type(query_hash:, options:) end - query_hash[:highlight] = highlight_options(options[:in]) if include_highlight?(options) - search(query_hash, options) end @@ -58,12 +56,6 @@ def preload_indexing_data(relation) attr_reader :noteable_type_to_feature - def include_highlight?(options) - return false if options[:count_only] || options[:noteable_type] - - true - end - def get_authorization_filter(query_hash, options) if use_new_auth?(options[:current_user]) options[:features] = noteable_type_to_feature.values diff --git a/ee/lib/gitlab/elastic/search_results.rb b/ee/lib/gitlab/elastic/search_results.rb index b9238fce5e8303970c62e729e58772e378af9fa7..440e8d31e8f8e3656afffbaa725c47f41d9275b7 100644 --- a/ee/lib/gitlab/elastic/search_results.rb +++ b/ee/lib/gitlab/elastic/search_results.rb @@ -453,7 +453,7 @@ def notes(count_only: false) def related_ids_for_notes(noteable_type) strong_memoize_with(:related_ids_for_notes, noteable_type) do - options = scope_options(:notes).merge(count_only: false, noteable_type: noteable_type) + options = scope_options(:notes).merge(count_only: false, noteable_type: noteable_type, related_ids_only: true) notes_response = Note.elastic_search(query, options: options).response notes_response['hits']['hits'].filter_map { |hit| hit['_source']['noteable_id'] }.uniq diff --git a/ee/lib/search/elastic/filters.rb b/ee/lib/search/elastic/filters.rb index 6ea65edb51c3ea6fbe77c2ef23befd977d5e596d..144b0f8f47d43e4d09fe5480ac9f7688f8a7147f 100644 --- a/ee/lib/search/elastic/filters.rb +++ b/ee/lib/search/elastic/filters.rb @@ -1031,8 +1031,12 @@ def by_noteable_type(query_hash:, options:) end context.name(:filters) do - query_hash[:_source] = ['noteable_id'] - query_hash[:size] = options.fetch(:related_size, DEFAULT_RELATED_SIZE) + if options[:related_ids_only] + query_hash[:_source] = ['noteable_id'] + # basic_query_hash automatically adds highlight + query_hash.delete(:highlight) + query_hash[:size] = options.fetch(:related_size, DEFAULT_RELATED_SIZE) + end add_filter(query_hash, :query, :bool, :filter) do { diff --git a/ee/spec/lib/elastic/latest/note_class_proxy_spec.rb b/ee/spec/lib/elastic/latest/note_class_proxy_spec.rb index 0740c40964f53fe469a59f5941ec62b71bb96647..d2a3cb7bbc11abae041f54ce97d7319f53bdf08d 100644 --- a/ee/spec/lib/elastic/latest/note_class_proxy_spec.rb +++ b/ee/spec/lib/elastic/latest/note_class_proxy_spec.rb @@ -79,8 +79,8 @@ end end - context 'when options[:noteable_type] is set' do - let(:options) { base_options.merge(noteable_type: 'Issue') } + context 'when options[:noteable_type] and options[:related_ids_only] are set' do + let(:options) { base_options.merge(noteable_type: 'Issue', related_ids_only: true) } it 'filters by noteable_type and returns noteable_id only in the _source array' do hits = result.response['hits']['hits'] @@ -120,8 +120,8 @@ end end - context 'when options[:noteable_type] is set' do - let(:options) { base_options.merge(noteable_type: 'Issue') } + context 'when options[:noteable_type] and options[:related_ids_only] are set' do + let(:options) { base_options.merge(noteable_type: 'Issue', related_ids_only: true) } it 'filters by noteable_type and returns noteable_id only in the _source array' do expect(result.response['hits']['hits'].first['_source'].keys).to contain_exactly('noteable_id') @@ -158,8 +158,8 @@ end end - context 'when options[:noteable_type] is set' do - let(:options) { base_options.merge(noteable_type: 'Issue') } + context 'when options[:noteable_type] and options[:related_ids_only] are set' do + let(:options) { base_options.merge(noteable_type: 'Issue', related_ids_only: true) } it 'filters by noteable_type and returns noteable_id only in the _source array' do expect(result.response['hits']['hits'].first['_source'].keys).to contain_exactly('noteable_id') diff --git a/ee/spec/lib/search/elastic/filters_spec.rb b/ee/spec/lib/search/elastic/filters_spec.rb index 428f2ea9ed0f03b4138bfc0021c1cb422e40c2e4..a61aa61313d1d2cae81145657bd3675085af1bbf 100644 --- a/ee/spec/lib/search/elastic/filters_spec.rb +++ b/ee/spec/lib/search/elastic/filters_spec.rb @@ -3618,7 +3618,26 @@ def compare_query_bool_fields(actual, expected) end describe '.by_noteable_type' do - let(:query_hash) { { query: { bool: { filter: [] } } } } + # use a query (including highlight) like what NoteClassProxy uses + let(:query_hash) do + { + query: { + bool: { + must: [], + should: [], + filter: [] + } + }, + highlight: { + fields: { + note: {} + }, + number_of_fragments: 0, + pre_tags: [Elastic::Latest::GitClassProxy::HIGHLIGHT_START_TAG], + post_tags: [Elastic::Latest::GitClassProxy::HIGHLIGHT_END_TAG] + } + } + end context 'when search_level is global' do it 'returns the original query_hash without modifications' do @@ -3651,9 +3670,31 @@ def compare_query_bool_fields(actual, expected) end end - context 'when valid noteable_type is provided' do + context 'when only valid noteable_type is provided' do let(:options) { { search_level: 'project', noteable_type: 'Issue' } } + it 'keeps the highlight if set' do + described_class.by_noteable_type(query_hash: query_hash, options: options) + + expect(query_hash).to have_key(:highlight) + end + + it 'does not set _source to only include noteable_id' do + described_class.by_noteable_type(query_hash: query_hash, options: options) + + expect(query_hash).not_to have_key(:_source) + end + + it 'does not sets size' do + described_class.by_noteable_type(query_hash: query_hash, options: options) + + expect(query_hash).not_to have_key(:size) + end + end + + context 'when valid noteable_type and related_ids_only is provided' do + let(:options) { { search_level: 'project', noteable_type: 'Issue', related_ids_only: true } } + it 'does not provide highlight' do described_class.by_noteable_type(query_hash: query_hash, options: options)