Merge branch...

Merge branch '300750-add-missing-reviewers-information-to-merge-request-email-notifications' into 'master'

Add reviewer information to merge_when_pipeline_succeeds_email

See merge request gitlab-org/gitlab!55463

app/views/notify/_users_list.html.haml

+%tr
+  %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" }
+    = user_label
+  %td{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif; margin: 0; padding: 14px 0 0px 5px; font-size: 15px; line-height: 1.4; color: #333333; font-weight: 400; width: 75%; border-top-style: solid; border-top-color: #ededed; border-top-width: 1px; -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; mso-table-lspace: 0pt; mso-table-rspace: 0pt;" }
+    %ul.users-list{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif; font-size: 15px; line-height: 1.4; padding-right: 5px; -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; mso-table-lspace: 0pt; mso-table-rspace: 0pt;" }
+      - users.each do |user|
+        %li
+          %img.avatar{ alt: "Avatar", height: "24", src: avatar_icon_for_user(user, 24, only_path: false), style: "border-radius: 12px; max-width: 100%; height: auto; -ms-interpolation-mode: bicubic; margin: -2px 0;", width: "24" }
+            %a.muted{ href: user_url(user), style: "color: #333333; text-decoration: none; -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; vertical-align: top;" }
+              = user.name

app/views/notify/merge_when_pipeline_succeeds_email.html.haml

@@ -42,13 +42,13 @@
           }
       }
 
-      ul.assignees-list {
+      ul.users-list {
           list-style: none;
           padding: 0px;
           display: block;
           margin-top: 0px;
       }
-      ul.assignees-list li {
+      ul.users-list li {
           display: inline-block;
           padding-right: 12px;
           padding-top: 8px;
@@ -137,16 +137,10 @@
                                               = @merge_request.author.name
 
                                 - if @merge_request.assignees.any?
-                                  %tr
-                                    %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" }
-                                      = assignees_label(@merge_request, include_value: false)
-                                    %td{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif; margin: 0; padding: 14px 0 0px 5px; font-size: 15px; line-height: 1.4; color: #333333; font-weight: 400; width: 75%; border-top-style: solid; border-top-color: #ededed; border-top-width: 1px; -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; mso-table-lspace: 0pt; mso-table-rspace: 0pt;" }
-                                      %ul.assignees-list{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif; font-size: 15px; line-height: 1.4; padding-right: 5px; -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; mso-table-lspace: 0pt; mso-table-rspace: 0pt;" }
-                                        - @merge_request.assignees.each do |assignee|
-                                          %li
-                                            %img.avatar{ alt: "Avatar", height: "24", src: avatar_icon_for_user(assignee, 24, only_path: false), style: "border-radius: 12px; max-width: 100%; height: auto; -ms-interpolation-mode: bicubic; margin: -2px 0;", width: "24" }
-                                            %a.muted{ href: user_url(assignee), style: "color: #333333; text-decoration: none; -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; vertical-align: top;" }
-                                              = assignee.name
+                                  = render 'users_list', users: @merge_request.assignees, user_label: assignees_label(@merge_request, include_value: false)
+
+                                - if @merge_request.reviewers.any?
+                                  = render 'users_list', users: @merge_request.reviewers, user_label: reviewers_label(@merge_request, include_value: false)
 
         = render_if_exists 'layouts/mailer/additional_text'
 

app/views/notify/merge_when_pipeline_succeeds_email.text.haml

@@ -6,3 +6,4 @@ Merge Request url: #{project_merge_request_url(@merge_request.target_project, @m
 
 Author: #{sanitize_name(@merge_request.author_name)}
 = assignees_label(@merge_request)
+= reviewers_label(@merge_request)

changelogs/unreleased/300750-add-missing-reviewers-information-to-merge-request-email-notificat.yml

+---
+title: Add reviewers detail to merge when pipeline succeeds email
+merge_request: 55463
+author:
+type: added

spec/mailers/emails/merge_requests_spec.rb

@@ -6,37 +6,47 @@ require 'email_spec'
 RSpec.describe Emails::MergeRequests do
   include EmailSpec::Matchers
 
-  describe "#resolved_all_discussions_email" do
-    let(:user) { create(:user) }
-    let(:merge_request) { create(:merge_request) }
-    let(:current_user) { create(:user) }
-
-    subject { Notify.resolved_all_discussions_email(user.id, merge_request.id, current_user.id) }
-
-    it "includes the name of the resolver" do
-      expect(subject).to have_body_text current_user.name
-    end
+  let_it_be(:recipient) { create(:user) }
+  let_it_be(:current_user) { create(:user) }
+  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(:project) { create(:project, :repository) }
+  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
 
   describe "#merge_when_pipeline_succeeds_email" do
-    let(:user) { create(:user) }
-    let(:merge_request) { create(:merge_request) }
-    let(:current_user) { create(:user) }
-    let(:project) { create(:project, :repository) }
     let(:title) { "Merge request #{merge_request.to_reference} was scheduled to merge after pipeline succeeds by #{current_user.name}" }
 
-    subject { Notify.merge_when_pipeline_succeeds_email(user.id, merge_request.id, current_user.id) }
+    subject { Notify.merge_when_pipeline_succeeds_email(recipient.id, merge_request.id, current_user.id) }
 
     it "has required details" do
-      expect(subject).to have_content title
-      expect(subject).to have_content merge_request.to_reference
-      expect(subject).to have_content current_user.name
+      aggregate_failures do
+        expect(subject).to have_content title
+        expect(subject).to have_content merge_request.to_reference
+        expect(subject).to have_content current_user.name
+        expect(subject.html_part).to have_content(assignee.name)
+        expect(subject.text_part).to have_content(assignee.name)
+        expect(subject.html_part).to have_content(reviewer.name)
+        expect(subject.text_part).to have_content(reviewer.name)
+      end
+    end
+  end
+
+  describe "#resolved_all_discussions_email" do
+    subject { Notify.resolved_all_discussions_email(recipient.id, merge_request.id, current_user.id) }
+
+    it "includes the name of the resolver" do
+      expect(subject).to have_body_text current_user.name
     end
   end
 
   describe '#merge_requests_csv_email' do
-    let(:user) { create(:user) }
-    let(:project) { create(:project) }
     let(:merge_requests) { create_list(:merge_request, 10) }
     let(:export_status) do
       {
@@ -48,10 +58,10 @@ RSpec.describe Emails::MergeRequests do
 
     let(:csv_data) { MergeRequests::ExportCsvService.new(MergeRequest.all, project).csv_data }
 
-    subject { Notify.merge_requests_csv_email(user, project, csv_data, export_status) }
+    subject { Notify.merge_requests_csv_email(recipient, project, csv_data, export_status) }
 
     it { expect(subject.subject).to eq("#{project.name} | Exported merge requests") }
-    it { expect(subject.to).to contain_exactly(user.notification_email_for(project.group)) }
+    it { expect(subject.to).to contain_exactly(recipient.notification_email_for(project.group)) }
     it { expect(subject.html_part).to have_content("Your CSV export of 10 merge requests from project") }
     it { expect(subject.text_part).to have_content("Your CSV export of 10 merge requests from project") }