Merge branch '247888-quoted-search-term-breaks-code-highlighting' into 'master'

Resolve "Quoted search term breaks code highlighting"

See merge request gitlab-org/gitlab!45278

app/assets/javascripts/pages/search/show/search.js

 import $ from 'jquery';
+import setHighlightClass from 'ee_else_ce/search/highlight_blob_search_result';
 import initDeprecatedJQueryDropdown from '~/deprecated_jquery_dropdown';
 import { deprecatedCreateFlash as Flash } from '~/flash';
 import Api from '~/api';
@@ -6,7 +7,6 @@ import { __ } from '~/locale';
 import Project from '~/pages/projects/project';
 import { visitUrl } from '~/lib/utils/url_utility';
 import refreshCounts from './refresh_counts';
-import setHighlightClass from './highlight_blob_search_result';
 
 export default class Search {
   constructor() {

app/views/search/results/_blob_data.html.haml

@@ -7,4 +7,4 @@
           = search_blob_title(project, path)
     - if blob.data
       .file-content.code.term{ data: { qa_selector: 'file_text_content' } }
-        = render 'shared/file_highlight', blob: blob, first_line_number: blob.startline, blob_link: blob_link
+        = render 'shared/file_highlight', blob: blob, first_line_number: blob.startline, blob_link: blob_link, highlight_line: blob.highlight_line

app/views/shared/_file_highlight.html.haml

 #blob-content.file-content.code.js-syntax-highlight
+  -  offset = defined?(first_line_number) ? first_line_number : 1
   .line-numbers
     - if blob.data.present?
       - link_icon = sprite_icon('link', size: 12)
       - link = blob_link if defined?(blob_link)
       - blob.data.each_line.each_with_index do |_, index|
-        - offset = defined?(first_line_number) ? first_line_number : 1
         - i = index + offset
         -# We're not using `link_to` because it is too slow once we get to thousands of lines.
         %a.diff-line-num{ href: "#{link}#L#{i}", id: "L#{i}", 'data-line-number' => i }
           = link_icon
           = i
-  .blob-content{ data: { blob_id: blob.id, path: blob.path } }
+  - highlight = defined?(highlight_line) && highlight_line ? highlight_line - offset : nil
+  .blob-content{ data: { blob_id: blob.id, path: blob.path, highlight_line: highlight } }
     %pre.code.highlight
       %code
         = blob.present.highlight

ee/app/assets/javascripts/search/highlight_blob_search_result.js

+import setHighlightClass from '~/search/highlight_blob_search_result';
+
+export default () => {
+  const highlightLineClass = 'hll';
+  const contentBody = document.getElementById('content-body');
+  const blobs = contentBody.querySelectorAll('.blob-result');
+
+  // Supports Basic (backed by Gitaly) Search highlighting
+  setHighlightClass();
+
+  // Supports Advanced (backed by Elasticsearch) Search highlighting
+  blobs.forEach(blob => {
+    const lines = blob.querySelectorAll('.line');
+    const dataHighlightLine = blob.querySelector('[data-highlight-line]');
+    if (dataHighlightLine) {
+      const { highlightLine } = dataHighlightLine.dataset;
+      lines[highlightLine].classList.add(highlightLineClass);
+    }
+  });
+};

ee/changelogs/unreleased/247888-quoted-search-term-breaks-code-highlighting.yml

+---
+title: Fix quoted search term code highlighting
+merge_request: 45278
+author:
+type: fixed

ee/lib/gitlab/elastic/search_results.rb

@@ -142,10 +142,12 @@ module Gitlab
         highlight_content = result.dig('highlight', 'blob.content')&.first || ''
 
         found_line_number = 0
+        highlight_found = false
 
         highlight_content.each_line.each_with_index do |line, index|
           if line.include?(::Elastic::Latest::GitClassProxy::HIGHLIGHT_START_TAG)
             found_line_number = index
+            highlight_found = true
             break
           end
         end
@@ -163,12 +165,15 @@ module Gitlab
              end
 
         data = content.lines[from..to]
+        # only send highlighted line number if a highlight was returned by Elasticsearch
+        highlight_line = highlight_found ? found_line_number + 1 : nil
 
         ::Gitlab::Search::FoundBlob.new(
           path: path,
           basename: basename,
           ref: ref,
           startline: from + 1,
+          highlight_line: highlight_line,
           data: data.join,
           project: project,
           project_id: project_id

ee/spec/frontend/fixtures/search.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe SearchController, '(JavaScript fixtures)', type: :controller do
+  include JavaScriptFixturesHelpers
+
+  render_views
+
+  let_it_be(:user) { create(:admin) }
+
+  before(:all) do
+    clean_frontend_fixtures('ee/search/')
+  end
+
+  before do
+    sign_in(user)
+  end
+
+  context 'search within a project', :sidekiq_inline do
+    let(:namespace) { create(:namespace, name: 'frontend-fixtures') }
+    let(:project) { create(:project, :public, :repository, namespace: namespace, path: 'search-project') }
+    let(:blobs) do
+      Kaminari.paginate_array([
+        Gitlab::Search::FoundBlob.new(
+          path: 'CHANGELOG',
+          basename: 'CHANGELOG',
+          ref: 'master',
+          data: "hello\nworld\nfoo\nbar # this is the highlight\nbaz\nboo\nbat",
+          project: project,
+          project_id: project.id,
+          startline: 2,
+          highlight_line: 4),
+        Gitlab::Search::FoundBlob.new(
+          path: 'CONTRIBUTING',
+          basename: 'CONTRIBUTING',
+          ref: 'master',
+          data: "hello\nworld\nfoo\nbar # this is the highlight\nbaz\nboo\nbat",
+          project: project,
+          project_id: project.id,
+          startline: 2,
+          highlight_line: 4),
+        Gitlab::Search::FoundBlob.new(
+          path: 'README',
+          basename: 'README',
+          ref: 'master',
+          data: "foo\nbar # this is the highlight\nbaz\nboo\nbat",
+          project: project,
+          project_id: project.id,
+          startline: 2,
+          highlight_line: 2)
+        ],
+        total_count: 3,
+        limit: 3,
+        offset: 0)
+    end
+
+    it 'ee/search/blob_search_result.html' do
+      expect_next_instance_of(SearchService) do |search_service|
+        expect(search_service).to receive(:search_objects).and_return(blobs)
+      end
+
+      get :show, params: {
+          search: 'Send',
+          project_id: project.id,
+          scope: :blobs
+      }
+
+      expect(response).to be_successful
+    end
+  end
+end

ee/spec/frontend/search/highlight_blob_search_result_spec.js

+import setHighlightClass from 'ee/search/highlight_blob_search_result';
+
+const fixture = 'ee/search/blob_search_result.html';
+const ceFixture = 'search/blob_search_result.html';
+
+describe('ee/search/highlight_blob_search_result', () => {
+  preloadFixtures(fixture, ceFixture);
+
+  // Basic search support
+  it('highlights lines with search term occurrence', () => {
+    loadFixtures(ceFixture);
+
+    setHighlightClass();
+
+    expect(document.querySelectorAll('.blob-result .hll').length).toBe(4);
+  });
+
+  // Advanced search support
+  it('highlights lines which have been identified by Elasticsearch', () => {
+    loadFixtures(fixture);
+
+    setHighlightClass();
+
+    expect(document.querySelectorAll('.blob-result .hll').length).toBe(3);
+  });
+});

ee/spec/lib/gitlab/elastic/search_results_spec.rb

@@ -101,6 +101,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
       expect(parsed).to be_kind_of(::Gitlab::Search::FoundBlob)
       expect(parsed).to have_attributes(
         startline: 1,
+        highlight_line: nil,
         project: project,
         data: "foo\n"
       )
@@ -123,6 +124,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
         basename: 'path/file',
         ref: 'sha',
         startline: 2,
+        highlight_line: 2,
         project: project,
         data: "bar\n"
       )
@@ -176,6 +178,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
           basename: 'path/file',
           ref: 'sha',
           startline: 2,
+          highlight_line: 4,
           project: project,
           data: expected_data
         )

lib/gitlab/search/found_blob.rb

@@ -9,7 +9,7 @@ module Gitlab
       include Gitlab::Utils::StrongMemoize
       include BlobActiveModel
 
-      attr_reader :project, :content_match, :blob_path
+      attr_reader :project, :content_match, :blob_path, :highlight_line
 
       PATH_REGEXP = /\A(?<ref>[^:]*):(?<path>[^\x00]*)\x00/.freeze
       CONTENT_REGEXP = /^(?<ref>[^:]*):(?<path>[^\x00]*)\x00(?<startline>\d+)\x00/.freeze
@@ -26,6 +26,7 @@ module Gitlab
         @binary_basename = opts.fetch(:basename, nil)
         @ref = opts.fetch(:ref, nil)
         @startline = opts.fetch(:startline, nil)
+        @highlight_line = opts.fetch(:highlight_line, nil)
         @binary_data = opts.fetch(:data, nil)
         @per_page = opts.fetch(:per_page, 20)
         @project = opts.fetch(:project, nil)

spec/frontend/fixtures/search.rb

@@ -26,8 +26,51 @@ RSpec.describe SearchController, '(JavaScript fixtures)', type: :controller do
   context 'search within a project' do
     let(:namespace) { create(:namespace, name: 'frontend-fixtures') }
     let(:project) { create(:project, :public, :repository, namespace: namespace, path: 'search-project') }
+    let(:blobs) do
+      Kaminari.paginate_array([
+        Gitlab::Search::FoundBlob.new(
+          path: 'CHANGELOG',
+          basename: 'CHANGELOG',
+          ref: 'master',
+          data: "hello\nworld\nfoo\nSend # this is the highligh\nbaz\nboo\nbat",
+          project: project,
+          project_id: project.id,
+          startline: 2),
+        Gitlab::Search::FoundBlob.new(
+          path: 'CONTRIBUTING',
+          basename: 'CONTRIBUTING',
+          ref: 'master',
+          data: "hello\nworld\nfoo\nSend # this is the highligh\nbaz\nboo\nbat",
+          project: project,
+          project_id: project.id,
+          startline: 2),
+        Gitlab::Search::FoundBlob.new(
+          path: 'README',
+          basename: 'README',
+          ref: 'master',
+          data: "foo\nSend # this is the highlight\nbaz\nboo\nbat",
+          project: project,
+          project_id: project.id,
+          startline: 2),
+        Gitlab::Search::FoundBlob.new(
+          path: 'test',
+          basename: 'test',
+          ref: 'master',
+          data: "foo\nSend # this is the highlight\nbaz\nboo\nbat",
+          project: project,
+          project_id: project.id,
+          startline: 2)
+     ],
+     total_count: 4,
+     limit: 4,
+     offset: 0)
+    end
 
     it 'search/blob_search_result.html' do
+      expect_next_instance_of(SearchService) do |search_service|
+        expect(search_service).to receive(:search_objects).and_return(blobs)
+      end
+
       get :show, params: {
         search: 'Send',
         project_id: project.id,

spec/frontend/pages/search/show/highlight_blob_search_result_spec.js → spec/frontend/search/highlight_blob_search_result_spec.js

-import setHighlightClass from '~/pages/search/show/highlight_blob_search_result';
+import setHighlightClass from '~/search/highlight_blob_search_result';
 
 const fixture = 'search/blob_search_result.html';
 
-describe('pages/search/show/highlight_blob_search_result', () => {
+describe('search/highlight_blob_search_result', () => {
   preloadFixtures(fixture);
 
   beforeEach(() => loadFixtures(fixture));
@@ -10,6 +10,6 @@ describe('pages/search/show/highlight_blob_search_result', () => {
   it('highlights lines with search term occurrence', () => {
     setHighlightClass();
 
-    expect(document.querySelectorAll('.blob-result .hll').length).toBe(11);
+    expect(document.querySelectorAll('.blob-result .hll').length).toBe(4);
   });
 });

spec/frontend/search_spec.js

 import $ from 'jquery';
+import setHighlightClass from 'ee_else_ce/search/highlight_blob_search_result';
 import Api from '~/api';
 import Search from '~/pages/search/show/search';
-import setHighlightClass from '~/pages/search/show/highlight_blob_search_result';
 
 jest.mock('~/api');
-jest.mock('~/pages/search/show/highlight_blob_search_result');
+jest.mock('ee_else_ce/search/highlight_blob_search_result');
 
 describe('Search', () => {
   const fixturePath = 'search/show.html';