Commit 68c15872 authored by David Kim's avatar David Kim Committed by Luke Duncalfe

Add reviewer information to new merge request email notification

parent fe676e7b
......@@ -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
......
......@@ -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
......
......@@ -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 %>
---
title: Add reviewers detail to new merge request email
merge_request: 54781
author:
type: added
......@@ -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
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment