Extract shared view search/results/_issuable.html.haml

At present the merge request result page has an old design and we want
it to match the issue design which was recently updated.

It seems the best way to do this was to share code so we've extracted a
shared HAML template for both of these.

app/controllers/search_controller.rb

@@ -9,6 +9,7 @@ class SearchController < ApplicationController
   SCOPE_PRELOAD_METHOD = {
     projects: :with_web_entity_associations,
     issues: :with_web_entity_associations,
+    merge_requests: :with_web_entity_associations,
     epics: :with_web_entity_associations
   }.freeze
 

app/helpers/search_helper.rb

@@ -367,10 +367,10 @@ module SearchHelper
   end
 
   # _search_highlight is used in EE override
-  def highlight_and_truncate_issue(issue, search_term, _search_highlight)
-    return unless issue.description.present?
+  def highlight_and_truncate_issuable(issuable, search_term, _search_highlight)
+    return unless issuable.description.present?
 
-    simple_search_highlight_and_truncate(issue.description, search_term, highlighter: '<span class="gl-text-black-normal gl-font-weight-bold">\1</span>')
+    simple_search_highlight_and_truncate(issuable.description, search_term, highlighter: '<span class="gl-text-black-normal gl-font-weight-bold">\1</span>')
   end
 
   def show_user_search_tab?
@@ -380,6 +380,36 @@ module SearchHelper
       can?(current_user, :read_users_list)
     end
   end
+
+  def issuable_state_to_badge_class(issuable)
+    # Closed is considered "danger" for MR so we need to handle separately
+    if issuable.is_a?(::MergeRequest)
+      if issuable.merged?
+        :primary
+      elsif issuable.closed?
+        :danger
+      else
+        :success
+      end
+    else
+      if issuable.closed?
+        :info
+      else
+        :success
+      end
+    end
+  end
+
+  def issuable_state_text(issuable)
+    case issuable.state
+    when 'merged'
+      _("Merged")
+    when 'closed'
+      _("Closed")
+    else
+      _("Open")
+    end
+  end
 end
 
 SearchHelper.prepend_if_ee('EE::SearchHelper')

app/models/merge_request.rb

@@ -294,6 +294,7 @@ class MergeRequest < ApplicationRecord
   scope :preload_author, -> { preload(:author) }
   scope :preload_approved_by_users, -> { preload(:approved_by_users) }
   scope :preload_metrics, -> (relation) { preload(metrics: relation) }
+  scope :with_web_entity_associations, -> { preload(:author, :target_project) }
 
   scope :with_auto_merge_enabled, -> do
     with_state(:opened).where(auto_merge_enabled: true)

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

+%div{ class: 'search-result-row gl-pb-3! gl-mt-5 gl-mb-0!' }
+  %span.gl-display-flex.gl-align-items-center
+    %span.badge.badge-pill.gl-badge.sm{ class: "badge-#{issuable_state_to_badge_class(issuable)}" }= issuable_state_text(issuable)
+    = sprite_icon('eye-slash', css_class: 'gl-text-gray-500 gl-ml-2') if issuable.respond_to?(:confidential?) && issuable.confidential?
+    = link_to issuable_path(issuable), data: { track_event: 'click_text', track_label: "#{issuable.class.name.downcase}_title", track_property: 'search_result' }, class: 'gl-w-full' do
+      %span.term.str-truncated.gl-font-weight-bold.gl-ml-2= issuable.title
+  .gl-text-gray-500.gl-my-3
+    = sprintf(s_(' %{project_name}#%{issuable_iid} · opened %{issuable_created} by %{author}'), { project_name: issuable.project.full_name, issuable_iid: issuable.iid, issuable_created: time_ago_with_tooltip(issuable.created_at, placement: 'bottom'), author: link_to_member(@project, issuable.author, avatar: false) }).html_safe
+  .description.term.col-sm-10.gl-px-0
+    = highlight_and_truncate_issuable(issuable, @search_term, @search_highlight)

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

-%div{ class: 'search-result-row gl-pb-3! gl-mt-5 gl-mb-0!' }
-  %span.gl-display-flex.gl-align-items-center
-    - if issue.closed?
-      %span.badge.badge-info.badge-pill.gl-badge.sm= _("Closed")
-    - else
-      %span.badge.badge-success.badge-pill.gl-badge.sm= _("Open")
-    = sprite_icon('eye-slash', css_class: 'gl-text-gray-500 gl-ml-2') if issue.confidential?
-    = link_to project_issue_path(issue.project, issue), data: { track_event: 'click_text', track_label: 'issue_title', track_property: 'search_result' }, class: 'gl-w-full' do
-      %span.term.str-truncated.gl-font-weight-bold.gl-ml-2= issue.title
-  .gl-text-gray-500.gl-my-3
-    = sprintf(s_(' %{project_name}#%{issue_iid} · opened %{issue_created} by %{author}'), { project_name: issue.project.full_name, issue_iid: issue.iid, issue_created: time_ago_with_tooltip(issue.created_at, placement: 'bottom'), author: link_to_member(@project, issue.author, avatar: false) }).html_safe
-  .description.term.col-sm-10.gl-px-0
-    = highlight_and_truncate_issue(issue, @search_term, @search_highlight)
+= render partial: 'search/results/issuable', object: issue

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

-.search-result-row
-  %h4
-    = link_to project_merge_request_path(merge_request.target_project, merge_request), data: {track_event: 'click_text', track_label: 'merge_request_title', track_property: 'search_result'} do
-      %span.term.str-truncated= merge_request.title
-    - if merge_request.merged?
-      %span.badge.badge-primary.gl-ml-2= _("Merged")
-    - elsif merge_request.closed?
-      %span.badge.badge-danger.gl-ml-2= _("Closed")
-    .float-right= merge_request.to_reference
-  - if merge_request.description.present?
-    .description.term
-      = search_md_sanitize(merge_request.description)
-  %span.light
-    #{merge_request.project.full_name}
+= render partial: 'search/results/issuable', object: merge_request

changelogs/unreleased/275899-share-view-code-for-mr-issue-search-results.yml

+---
+title: Update merge request search results design
+merge_request: 46944
+author:
+type: changed

ee/app/helpers/ee/search_helper.rb

@@ -61,14 +61,14 @@ module EE
       end
     end
 
-    override :highlight_and_truncate_issue
-    def highlight_and_truncate_issue(issue, search_term, search_highlight)
-      return super unless search_service.use_elasticsearch? && search_highlight[issue.id]&.description.present?
+    override :highlight_and_truncate_issuable
+    def highlight_and_truncate_issuable(issuable, search_term, search_highlight)
+      return super unless search_service.use_elasticsearch? && search_highlight[issuable.id]&.description.present?
 
       # We use Elasticsearch highlighting for results from Elasticsearch. Sanitize the description, replace the
       # pre/post tags from Elasticsearch with highlighting, truncate, and mark as html_safe. HTML tags are not
       # counted towards the character limit.
-      text = sanitize(search_highlight[issue.id].description.first)
+      text = sanitize(search_highlight[issuable.id].description.first)
       text.gsub!(::Elastic::Latest::GitClassProxy::HIGHLIGHT_START_TAG, '<span class="gl-text-black-normal gl-font-weight-bold">')
       text.gsub!(::Elastic::Latest::GitClassProxy::HIGHLIGHT_END_TAG, '</span>')
       Truncato.truncate(text, count_tags: false, count_tail: false, max_length: 200).html_safe

ee/spec/helpers/search_helper_spec.rb

@@ -148,7 +148,7 @@ RSpec.describe SearchHelper do
     end
   end
 
-  describe '#highlight_and_truncate_issue' do
+  describe '#highlight_and_truncate_issuable' do
     let(:description) { 'hello world' }
     let(:issue) { create(:issue, description: description) }
     let(:user) { create(:user) }
@@ -160,7 +160,7 @@ RSpec.describe SearchHelper do
     end
 
     # Elasticsearch returns Elasticsearch::Model::HashWrapper class for the highlighting
-    subject { highlight_and_truncate_issue(issue, 'test', Elasticsearch::Model::HashWrapper.new(search_highlight)) }
+    subject { highlight_and_truncate_issuable(issue, 'test', Elasticsearch::Model::HashWrapper.new(search_highlight)) }
 
     context 'when description is not present' do
       let(:description) { nil }

locale/gitlab.pot

@@ -16,7 +16,7 @@ msgstr ""
 "Content-Transfer-Encoding: 8bit\n"
 "Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n"
 
-msgid " %{project_name}#%{issue_iid} · opened %{issue_created} by %{author}"
+msgid " %{project_name}#%{issuable_iid} · opened %{issuable_created} by %{author}"
 msgstr ""
 
 msgid " %{start} to %{end}"

spec/helpers/search_helper_spec.rb

@@ -477,7 +477,7 @@ RSpec.describe SearchHelper do
     end
   end
 
-  describe '#highlight_and_truncate_issue' do
+  describe '#highlight_and_truncate_issuable' do
     let(:description) { 'hello world' }
     let(:issue) { create(:issue, description: description) }
     let(:user) { create(:user) }
@@ -486,7 +486,7 @@ RSpec.describe SearchHelper do
       allow(self).to receive(:current_user).and_return(user)
     end
 
-    subject { highlight_and_truncate_issue(issue, 'test', {}) }
+    subject { highlight_and_truncate_issuable(issue, 'test', {}) }
 
     context 'when description is not present' do
       let(:description) { nil }
@@ -545,4 +545,38 @@ RSpec.describe SearchHelper do
       end
     end
   end
+
+  describe '#issuable_state_to_badge_class' do
+    context 'with merge request' do
+      it 'returns correct badge based on status' do
+        expect(issuable_state_to_badge_class(build(:merge_request, :merged))).to eq(:primary)
+        expect(issuable_state_to_badge_class(build(:merge_request, :closed))).to eq(:danger)
+        expect(issuable_state_to_badge_class(build(:merge_request, :opened))).to eq(:success)
+      end
+    end
+
+    context 'with an issue' do
+      it 'returns correct badge based on status' do
+        expect(issuable_state_to_badge_class(build(:issue, :closed))).to eq(:info)
+        expect(issuable_state_to_badge_class(build(:issue, :opened))).to eq(:success)
+      end
+    end
+  end
+
+  describe '#issuable_state_text' do
+    context 'with merge request' do
+      it 'returns correct badge based on status' do
+        expect(issuable_state_text(build(:merge_request, :merged))).to eq(_('Merged'))
+        expect(issuable_state_text(build(:merge_request, :closed))).to eq(_('Closed'))
+        expect(issuable_state_text(build(:merge_request, :opened))).to eq(_('Open'))
+      end
+    end
+
+    context 'with an issue' do
+      it 'returns correct badge based on status' do
+        expect(issuable_state_text(build(:issue, :closed))).to eq(_('Closed'))
+        expect(issuable_state_text(build(:issue, :opened))).to eq(_('Open'))
+      end
+    end
+  end
 end

spec/requests/search_controller_spec.rb

@@ -45,14 +45,9 @@ RSpec.describe SearchController, type: :request do
       let(:object) { :merge_request }
       let(:creation_args) { { source_project: project, title: 'bar' } }
       let(:params) { { search: 'bar', scope: 'merge_requests' } }
-      # some N+1 queries exist
-      # each merge request require 4 extra queries for:
-      # - one for projects
-      # - one for namespaces
-      # - two for routes
-      # plus 4 additional queries run for the logged in user:
+      # there are 4 additional queries run for the logged in user:
       #   - (1) geo_nodes, (1) users, (2) broadcast_messages
-      let(:threshold) { 16 }
+      let(:threshold) { 4 }
 
       it_behaves_like 'an efficient database result'
     end