Merge branch...

Merge branch '300750-merge-request-email-notifications-don-t-include-reviewer-information' into 'master'

Add reviewer information to new merge request email notification

See merge request gitlab-org/gitlab!54781

app/helpers/merge_requests_helper.rb

@@ -174,6 +174,18 @@ module MergeRequestsHelper
     end
   end
 
+  def reviewers_label(merge_request, include_value: true)
+    reviewers = merge_request.reviewers
+    reviewer_label = 'Reviewer'.pluralize(reviewers.count)
+
+    if include_value
+      sanitized_list = sanitize_name(reviewers.map(&:name).to_sentence)
+      "#{reviewer_label}: #{sanitized_list}"
+    else
+      reviewer_label
+    end
+  end
+
   private
 
   def review_requested_merge_requests_count

app/views/notify/new_merge_request_email.html.haml

@@ -8,6 +8,8 @@
     Author: #{@merge_request.author_name}
   .assignee
     = assignees_label(@merge_request)
+  .reviewer
+    = reviewers_label(@merge_request)
   .approvers
     = render_if_exists 'notify/merge_request_approvers', presenter: @mr_presenter
 

app/views/notify/new_merge_request_email.text.erb

@@ -3,6 +3,7 @@
 <%= merge_path_description(@merge_request, 'to') %>
 <%= 'Author:' %>    <%= @merge_request.author_name %>
 <%= assignees_label(@merge_request) if @merge_request.assignees.any? %>
+<%= reviewers_label(@merge_request) if @merge_request.reviewers.any? %>
 <%= render_if_exists 'notify/merge_request_approvers', presenter: @mr_presenter %>
 
 <%= @merge_request.description %>

changelogs/unreleased/300750-merge-request-email-notifications-don-t-include-reviewer-informati.yml

+---
+title: Add reviewers detail to new merge request email
+merge_request: 54781
+author:
+type: added

spec/helpers/merge_requests_helper_spec.rb

@@ -90,4 +90,57 @@ RSpec.describe MergeRequestsHelper do
       )
     end
   end
+
+  describe '#reviewers_label' do
+    let(:merge_request) { build_stubbed(:merge_request) }
+    let(:reviewer1) { build_stubbed(:user, name: 'Jane Doe') }
+    let(:reviewer2) { build_stubbed(:user, name: 'John Doe') }
+
+    before do
+      allow(merge_request).to receive(:reviewers).and_return(reviewers)
+    end
+
+    context 'when multiple reviewers exist' do
+      let(:reviewers) { [reviewer1, reviewer2] }
+
+      it 'returns reviewer label with reviewer names' do
+        expect(helper.reviewers_label(merge_request)).to eq("Reviewers: Jane Doe and John Doe")
+      end
+
+      it 'returns reviewer label only with include_value: false' do
+        expect(helper.reviewers_label(merge_request, include_value: false)).to eq("Reviewers")
+      end
+
+      context 'when the name contains a URL' do
+        let(:reviewers) { [build_stubbed(:user, name: 'www.gitlab.com')] }
+
+        it 'returns sanitized name' do
+          expect(helper.reviewers_label(merge_request)).to eq("Reviewer: www_gitlab_com")
+        end
+      end
+    end
+
+    context 'when one reviewer exists' do
+      let(:reviewers) { [reviewer1] }
+
+      it 'returns reviewer label with no names' do
+        expect(helper.reviewers_label(merge_request)).to eq("Reviewer: Jane Doe")
+      end
+
+      it 'returns reviewer label only with include_value: false' do
+        expect(helper.reviewers_label(merge_request, include_value: false)).to eq("Reviewer")
+      end
+    end
+
+    context 'when no reviewers exist' do
+      let(:reviewers) { [] }
+
+      it 'returns reviewer label with no names' do
+        expect(helper.reviewers_label(merge_request)).to eq("Reviewers: ")
+      end
+      it 'returns reviewer label only with include_value: false' do
+        expect(helper.reviewers_label(merge_request, include_value: false)).to eq("Reviewers")
+      end
+    end
+  end
 end

spec/mailers/notify_spec.rb

@@ -16,12 +16,14 @@ RSpec.describe Notify do
   let_it_be(:user, reload: true) { create(:user) }
   let_it_be(:current_user, reload: true) { create(:user, email: "current@email.com", name: 'www.example.com') }
   let_it_be(:assignee, reload: true) { create(:user, email: 'assignee@example.com', name: 'John Doe') }
+  let_it_be(:reviewer, reload: true) { create(:user, email: 'reviewer@example.com', name: 'Jane Doe') }
 
   let_it_be(:merge_request) do
     create(:merge_request, source_project: project,
                            target_project: project,
                            author: current_user,
                            assignees: [assignee],
+                           reviewers: [reviewer],
                            description: 'Awesome description')
   end
 
@@ -342,6 +344,7 @@ RSpec.describe Notify do
             is_expected.to have_body_text(project_merge_request_path(project, merge_request))
             is_expected.to have_body_text(merge_request.source_branch)
             is_expected.to have_body_text(merge_request.target_branch)
+            is_expected.to have_body_text(reviewer.name)
           end
         end