Merge branch 'huacnlee/gitlab-ce-feature/option-to-include-author-info-in-mail' into 'master'

Add option to include the sender name in body of Notify email

_Originally opened at !2495 by @huacnlee._

- - -

Fixes #7824.

See merge request !2767

CHANGELOG

@@ -10,6 +10,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

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: []
     )

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

app/views/admin/application_settings/_form.html.haml

@@ -47,6 +47,16 @@
           = f.label :version_check_enabled do
             = 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

app/views/notify/_note_message.html.haml

+- 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)

app/views/notify/new_issue_email.html.haml

+- 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)
 

app/views/notify/new_merge_request_email.html.haml

+- 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, '→')
 

db/migrate/20160121030729_add_email_author_in_body_to_application_settings.rb

+class AddEmailAuthorInBodyToApplicationSettings < ActiveRecord::Migration
+  def change
+    add_column :application_settings, :email_author_in_body, :boolean, default: false
+  end
+end

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|

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