Add option to include the sender name in body of Notify email. fix #7824huacnlee/gitlab-ce-feature/option-to-include-author-info-in-mail

10 files changed, 65 insertions, 0 deletions

diff --git a/CHANGELOG b/CHANGELOG
index c0562feafca..664b543bea1 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -9,6 +9,7 @@ v 8.5.0 (unreleased)
   - Render sanitized SVG images (Stan Hu)
   - Support download access by PRIVATE-TOKEN header (Stan Hu)
   - Upgrade gitlab_git to 7.2.23 to fix commit message mentions in first branch push
+  - Add option to include the sender name in body of Notify email (Jason Lee)
   - New UI for pagination
   - Don't prevent sign out when 2FA enforcement is enabled and user hasn't yet
     set it up
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 1515086b16d..04a99d8c84a 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -81,6 +81,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :sentry_dsn,
       :akismet_enabled,
       :akismet_api_key,
+      :email_author_in_body,
       restricted_visibility_levels: [],
       import_sources: []
     )
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 9cafc78f761..44bbe5fb168 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -43,6 +43,7 @@
 #  metrics_port                      :integer          default(8089)
 #  sentry_enabled                    :boolean          default(FALSE)
 #  sentry_dsn                        :string
+#  email_author_in_body              :boolean          default(FALSE)
 #
 
 class ApplicationSetting < ActiveRecord::Base
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index b4e3d96d405..b30dfd109ea 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -48,6 +48,16 @@
             = f.check_box :version_check_enabled
             Version check enabled
     .form-group
+      .col-sm-offset-2.col-sm-10
+        .checkbox
+          = f.label :email_author_in_body do
+            = f.check_box :email_author_in_body
+            Include author name in notification email body
+          .help-block
+            Some email servers do not support overriding the email sender name.
+            Enable this option to include the name of the author of the issue,
+            merge request or comment in the email body instead.
+    .form-group
       = f.label :admin_notification_email, class: 'control-label col-sm-2'
       .col-sm-10
         = f.text_field :admin_notification_email, class: 'form-control'
diff --git a/app/views/notify/_note_message.html.haml b/app/views/notify/_note_message.html.haml
index 00cb4aa24cc..12ded41fbf2 100644
--- a/app/views/notify/_note_message.html.haml
+++ b/app/views/notify/_note_message.html.haml
@@ -1,2 +1,5 @@
+- if current_application_settings.email_author_in_body
+  %div
+    #{link_to @note.author_name, user_url(@note.author)} wrote:
 %div
   = markdown(@note.note, pipeline: :email)
diff --git a/app/views/notify/new_issue_email.html.haml b/app/views/notify/new_issue_email.html.haml
index d3b799fca23..ad3ab2525bb 100644
--- a/app/views/notify/new_issue_email.html.haml
+++ b/app/views/notify/new_issue_email.html.haml
@@ -1,3 +1,6 @@
+- if current_application_settings.email_author_in_body
+  %div
+    #{link_to @issue.author_name, user_url(@issue.author)} wrote:
 -if @issue.description
   = markdown(@issue.description, pipeline: :email)
 
diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml
index 90ebdfc3fe2..23423e7d981 100644
--- a/app/views/notify/new_merge_request_email.html.haml
+++ b/app/views/notify/new_merge_request_email.html.haml
@@ -1,3 +1,6 @@
+- if current_application_settings.email_author_in_body
+  %div
+    #{link_to @merge_request.author_name, user_url(@merge_request.author)} wrote:
 %p.details
   != merge_path_description(@merge_request, '&rarr;')
 
diff --git a/db/migrate/20160121030729_add_email_author_in_body_to_application_settings.rb b/db/migrate/20160121030729_add_email_author_in_body_to_application_settings.rb
new file mode 100644
index 00000000000..d50791410f9
--- /dev/null
+++ b/db/migrate/20160121030729_add_email_author_in_body_to_application_settings.rb
@@ -0,0 +1,5 @@
+class AddEmailAuthorInBodyToApplicationSettings < ActiveRecord::Migration
+  def change
+    add_column :application_settings, :email_author_in_body, :boolean, default: false
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index d4710346b82..f2eb48e37ed 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -66,6 +66,7 @@ ActiveRecord::Schema.define(version: 20160129135155) do
     t.string   "sentry_dsn"
     t.boolean  "akismet_enabled",                   default: false
     t.string   "akismet_api_key"
+    t.boolean  "email_author_in_body",              default: false
   end
 
   create_table "audit_events", force: :cascade do |t|
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 7289e596ef3..82bd057b16c 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -270,6 +270,17 @@ describe Notify do
           it 'contains a link to the new issue' do
             is_expected.to have_body_text /#{namespace_project_issue_path project.namespace, project, issue}/
           end
+
+          context 'when enabled email_author_in_body' do
+            before do
+              allow(current_application_settings).to receive(:email_author_in_body).and_return(true)
+            end
+
+            it 'contains a link to note author' do
+              is_expected.to have_body_text issue.author_name
+              is_expected.to have_body_text /wrote\:/
+            end
+          end
         end
 
         describe 'that are new with a description' do
@@ -377,6 +388,17 @@ describe Notify do
           it 'has the correct message-id set' do
             is_expected.to have_header 'Message-ID', "<merge_request_#{merge_request.id}@#{Gitlab.config.gitlab.host}>"
           end
+
+          context 'when enabled email_author_in_body' do
+            before do
+              allow(current_application_settings).to receive(:email_author_in_body).and_return(true)
+            end
+
+            it 'contains a link to note author' do
+              is_expected.to have_body_text merge_request.author_name
+              is_expected.to have_body_text /wrote\:/
+            end
+          end
         end
 
         describe 'that are new with a description' do
@@ -550,6 +572,21 @@ describe Notify do
         it 'contains the message from the note' do
           is_expected.to have_body_text /#{note.note}/
         end
+
+        it 'not contains note author' do
+          is_expected.not_to have_body_text /wrote\:/
+        end
+
+        context 'when enabled email_author_in_body' do
+          before do
+            allow(current_application_settings).to receive(:email_author_in_body).and_return(true)
+          end
+
+          it 'contains a link to note author' do
+            is_expected.to have_body_text note.author_name
+            is_expected.to have_body_text /wrote\:/
+          end
+        end
       end
 
       describe 'on a commit' do