Commit 8f443234 authored by Jan Provaznik's avatar Jan Provaznik Committed by Robert Speicher

Use custom user name for service desk emails

Allows to set and use custom name for outgoing service desk
emails.
parent cf8ea254
......@@ -51,7 +51,7 @@ module Emails
add_project_headers
headers['X-GitLab-Author'] = @message.author_username
mail(from: sender(@message.author_id, @message.send_from_committer_email?),
mail(from: sender(@message.author_id, send_from_user_email: @message.send_from_committer_email?),
reply_to: @message.reply_to,
subject: @message.subject)
end
......
......@@ -59,11 +59,11 @@ class Notify < BaseMailer
# Return an email address that displays the name of the sender.
# Only the displayed name changes; the actual email address is always the same.
def sender(sender_id, send_from_user_email = false)
def sender(sender_id, send_from_user_email: false, sender_name: nil)
return unless sender = User.find(sender_id)
address = default_sender_address
address.display_name = sender.name
address.display_name = sender_name.presence || sender.name
if send_from_user_email && can_send_from_user_email?(sender)
address.address = sender.email
......
......@@ -10,13 +10,21 @@ class Projects::ServiceDeskController < Projects::ApplicationController
def update
Projects::UpdateService.new(project, current_user, { service_desk_enabled: params[:service_desk_enabled] }).execute
ServiceDeskSetting.update_template_key_for(project: project, issue_template_key: params[:issue_template_key])
result = ServiceDeskSettings::UpdateService.new(project, current_user, setting_params).execute
json_response
if result[:status] == :success
json_response
else
render json: { message: result[:message] }, status: :unprocessable_entity
end
end
private
def setting_params
params.permit(:issue_template_key, :outgoing_name)
end
def json_response
respond_to do |format|
service_desk_settings = project.service_desk_setting
......@@ -26,7 +34,8 @@ class Projects::ServiceDeskController < Projects::ApplicationController
service_desk_address: project.service_desk_address,
service_desk_enabled: project.service_desk_enabled,
issue_template_key: service_desk_settings&.issue_template_key,
template_file_missing: service_desk_settings&.issue_template_missing?
template_file_missing: service_desk_settings&.issue_template_missing?,
outgoing_name: service_desk_settings&.outgoing_name
}
format.json { render json: service_desk_attributes }
......
......@@ -11,13 +11,26 @@ module Emails
def service_desk_thank_you_email(issue_id)
setup_service_desk_mail(issue_id)
mail_new_thread(@issue, service_desk_options(@support_bot.id).merge(subject: "Re: #{@issue.title} (##{@issue.iid})"))
options = {
from: sender(@support_bot.id, send_from_user_email: false, sender_name: @project.service_desk_setting&.outgoing_name),
to: @issue.service_desk_reply_to,
subject: "Re: #{@issue.title} (##{@issue.iid})"
}
mail_new_thread(@issue, options)
end
def service_desk_new_note_email(issue_id, note_id)
@note = Note.find(note_id)
setup_service_desk_mail(issue_id)
mail_answer_thread(@issue, service_desk_options(@note.author_id).merge(subject: "#{@issue.title} (##{@issue.iid})"))
options = {
from: sender(@note.author_id),
to: @issue.service_desk_reply_to,
subject: "#{@issue.title} (##{@issue.iid})"
}
mail_answer_thread(@issue, options)
end
private
......@@ -29,12 +42,5 @@ module Emails
@sent_notification = SentNotification.record(@issue, @support_bot.id, reply_key)
end
def service_desk_options(author_id)
{
from: sender(author_id),
to: @issue.service_desk_reply_to
}
end
end
end
......@@ -8,15 +8,6 @@ class ServiceDeskSetting < ApplicationRecord
validate :valid_issue_template
validates :outgoing_name, length: { maximum: 255 }, allow_blank: true
def self.update_template_key_for(project:, issue_template_key:)
return unless issue_template_key
settings = safe_find_or_create_by!(project_id: project.id)
settings.update!(issue_template_key: issue_template_key)
settings
end
def issue_template_content
strong_memoize(:issue_template_content) do
next unless issue_template_key.present?
......@@ -32,7 +23,7 @@ class ServiceDeskSetting < ApplicationRecord
def valid_issue_template
if issue_template_missing?
errors.add(:issue_template_key, "Issue template empty or not found")
errors.add(:issue_template_key, "is empty or does not exist")
end
end
end
# frozen_string_literal: true
module ServiceDeskSettings
class UpdateService < BaseService
def execute
settings = ServiceDeskSetting.safe_find_or_create_by!(project_id: project.id)
if settings.update(params)
success
else
error(settings.errors.full_messages.to_sentence)
end
end
end
end
......@@ -3,8 +3,12 @@
require 'spec_helper'
describe Projects::ServiceDeskController do
let(:project) { create(:project_empty_repo, :private, service_desk_enabled: true) }
let(:user) { create(:user) }
let_it_be(:project) do
create(:project, :private, :custom_repo, service_desk_enabled: true,
files: { '.gitlab/issue_templates/service_desk.md' => 'template' })
end
let_it_be(:user) { create(:user) }
before do
allow(License).to receive(:feature_available?).and_call_original
......@@ -40,9 +44,7 @@ describe Projects::ServiceDeskController do
context 'when issue template is present' do
it 'returns template_file_missing as false' do
template_path = '.gitlab/issue_templates/service_desk.md'
project.repository.create_file(user, template_path, 'text from template', message: 'message', branch_name: 'master')
ServiceDeskSetting.update_template_key_for(project: project, issue_template_key: 'service_desk')
create(:service_desk_setting, project: project, issue_template_key: 'service_desk')
get :show, params: { namespace_id: project.namespace.to_param, project_id: project }, format: :json
......@@ -67,7 +69,9 @@ describe Projects::ServiceDeskController do
it 'toggles services desk incoming email' do
project.update!(service_desk_enabled: false)
put :update, params: { namespace_id: project.namespace.to_param, project_id: project, service_desk_enabled: true }, format: :json
put :update, params: { namespace_id: project.namespace.to_param,
project_id: project,
service_desk_enabled: true }, format: :json
expect(json_response["service_desk_address"]).to be_present
expect(json_response["service_desk_enabled"]).to be_truthy
......@@ -75,11 +79,9 @@ describe Projects::ServiceDeskController do
end
it 'sets issue_template_key' do
template_path = '.gitlab/issue_templates/service_desk.md'
project.repository.create_file(user, template_path, 'template text', message: 'message', branch_name: 'master')
ServiceDeskSetting.update_template_key_for(project: project, issue_template_key: 'service_desk')
put :update, params: { namespace_id: project.namespace.to_param, project_id: project, issue_template_key: 'service_desk' }, format: :json
put :update, params: { namespace_id: project.namespace.to_param,
project_id: project,
issue_template_key: 'service_desk' }, format: :json
settings = project.service_desk_setting
expect(settings).to be_present
......@@ -88,6 +90,15 @@ describe Projects::ServiceDeskController do
expect(json_response['issue_template_key']).to eq('service_desk')
end
it 'returns an error when update of service desk settings fails' do
put :update, params: { namespace_id: project.namespace.to_param,
project_id: project,
issue_template_key: 'invalid key' }, format: :json
expect(response.status).to eq(422)
expect(json_response['message']).to eq('Issue template key is empty or does not exist')
end
context 'when user cannot admin the project' do
let(:other_user) { create(:user) }
......
# frozen_string_literal: true
FactoryBot.define do
factory :service_desk_setting do
project
end
end
......@@ -61,10 +61,12 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
end
context 'and template is present' do
let_it_be(:settings) { create(:service_desk_setting, project: project) }
def set_template_file(file_name, content)
file_path = ".gitlab/issue_templates/#{file_name}.md"
project.repository.create_file(user, file_path, content, message: 'message', branch_name: 'master')
ServiceDeskSetting.update_template_key_for(project: project, issue_template_key: file_name)
settings.update!(issue_template_key: file_name)
end
it 'appends template text to issue description' do
......
......@@ -68,6 +68,12 @@ describe Notify do
issue.update!(service_desk_reply_to: 'service.desk@example.com')
end
def expect_sender(username)
sender = subject.header[:from].addrs[0]
expect(sender.display_name).to eq(username)
expect(sender.address).to eq(gitlab_sender)
end
describe 'thank you email' do
subject { described_class.service_desk_thank_you_email(issue.id) }
......@@ -83,6 +89,26 @@ describe Notify do
is_expected.to have_body_text("Thank you for your support request! We are tracking your request as ticket #{issue.to_reference}, and will respond as soon as we can.")
end
end
it 'uses service bot name by default' do
expect_sender(User.support_bot.name)
end
context 'when custom outgoing name is set' do
let_it_be(:settings) { create(:service_desk_setting, project: project, outgoing_name: 'some custom name') }
it 'uses custom name in "from" header' do
expect_sender('some custom name')
end
end
context 'when custom outgoing name is empty' do
let_it_be(:settings) { create(:service_desk_setting, project: project, outgoing_name: '') }
it 'uses service bot name' do
expect_sender(User.support_bot.name)
end
end
end
describe 'new note email' do
......@@ -96,6 +122,10 @@ describe Notify do
is_expected.to deliver_to('service.desk@example.com')
end
it 'uses author\'s name in "from" header' do
expect_sender(first_note.author.name)
end
it 'has the correct subject and body' do
aggregate_failures do
is_expected.to have_referable_subject(issue, include_project: false, reply: true)
......
......@@ -6,33 +6,26 @@ describe ServiceDeskSetting do
describe 'validations' do
it { is_expected.to validate_presence_of(:project_id) }
it { is_expected.to validate_length_of(:outgoing_name).is_at_most(255) }
end
describe 'associations' do
it { is_expected.to belong_to(:project) }
end
describe '.valid_issue_template' do
let_it_be(:project) { create(:project, :custom_repo, files: { '.gitlab/issue_templates/service_desk.md' => 'template' }) }
describe '.update_template_key_for' do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :repository) }
it 'is not valid if template does not exist' do
settings = build(:service_desk_setting, project: project, issue_template_key: 'invalid key')
context 'when template exists' do
it 'updates issue_template_key' do
template_path = '.gitlab/issue_templates/service_desk.md'
project.repository.create_file(user, template_path, 'Template text', message: 'message', branch_name: 'master')
expect(settings).not_to be_valid
expect(settings.errors[:issue_template_key].first).to eq('is empty or does not exist')
end
described_class.update_template_key_for(project: project, issue_template_key: 'service_desk')
it 'is valid if template exists' do
settings = build(:service_desk_setting, project: project, issue_template_key: 'service_desk')
expect(project.service_desk_setting.issue_template_key).to eq('service_desk')
expect(settings).to be_valid
end
end
end
context 'when template does not exist' do
it 'raises error' do
expect do
described_class.update_template_key_for(project: project, issue_template_key: 'unknown')
end.to raise_error(ActiveRecord::RecordInvalid)
end
end
describe 'associations' do
it { is_expected.to belong_to(:project) }
end
end
# frozen_string_literal: true
require 'spec_helper'
describe ServiceDeskSettings::UpdateService do
describe '#execute' do
let_it_be(:settings) { create(:service_desk_setting, outgoing_name: 'original name') }
let_it_be(:user) { create(:user) }
context 'with valid params' do
let(:params) { { outgoing_name: 'some name' } }
it 'updates service desk settings' do
result = described_class.new(settings.project, user, params).execute
expect(result[:status]).to eq :success
expect(settings.reload.outgoing_name).to eq 'some name'
end
end
context 'with invalid params' do
let(:params) { { outgoing_name: 'x' * 256 } }
it 'does not update service desk settings' do
result = described_class.new(settings.project, user, params).execute
expect(result[:status]).to eq :error
expect(result[:message]).to eq 'Outgoing name is too long (maximum is 255 characters)'
expect(settings.reload.outgoing_name).to eq 'original name'
end
end
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