diff --git a/app/views/notify/_users_list.html.haml b/app/views/notify/_users_list.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..3e436ea8765c566e42e4d514ecc6c95f3d94cf7a --- /dev/null +++ b/app/views/notify/_users_list.html.haml @@ -0,0 +1,10 @@ +%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 diff --git a/app/views/notify/merge_when_pipeline_succeeds_email.html.haml b/app/views/notify/merge_when_pipeline_succeeds_email.html.haml index 4db213fb2295c0d11374c1835612e9031fd9396c..e7c51c8fb1360affa47052a4a199cb1385a94b24 100644 --- a/app/views/notify/merge_when_pipeline_succeeds_email.html.haml +++ b/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' diff --git a/app/views/notify/merge_when_pipeline_succeeds_email.text.haml b/app/views/notify/merge_when_pipeline_succeeds_email.text.haml index fdc23a6af0fb94bfad31d0d11cc0c524c8136312..de29dda6c71e89b7f52dfce99c0a476897662e2c 100644 --- a/app/views/notify/merge_when_pipeline_succeeds_email.text.haml +++ b/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) diff --git a/changelogs/unreleased/300750-add-missing-reviewers-information-to-merge-request-email-notificat.yml b/changelogs/unreleased/300750-add-missing-reviewers-information-to-merge-request-email-notificat.yml new file mode 100644 index 0000000000000000000000000000000000000000..5b23c6204b6687151509b2261e3423d428f5b15e --- /dev/null +++ b/changelogs/unreleased/300750-add-missing-reviewers-information-to-merge-request-email-notificat.yml @@ -0,0 +1,5 @@ +--- +title: Add reviewers detail to merge when pipeline succeeds email +merge_request: 55463 +author: +type: added diff --git a/spec/mailers/emails/merge_requests_spec.rb b/spec/mailers/emails/merge_requests_spec.rb index 34665d943ab6a56bccaaf102ca0213cf7e3556a2..5ab0265203e8751c2203e8336f280cbdbaeed104 100644 --- a/spec/mailers/emails/merge_requests_spec.rb +++ b/spec/mailers/emails/merge_requests_spec.rb @@ -6,37 +6,47 @@ 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 @@ 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") }