From 49cb1c7277d4b26cba8f28745ca9b9e348c72a50 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Mon, 17 Feb 2014 16:25:40 +0100
Subject: [PATCH 1/5] Fix misleading comment

---
 app/mailers/notify.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 6c1a332896..e58db8ef35 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -18,7 +18,7 @@ class Notify < ActionMailer::Base
   default from: Gitlab.config.gitlab.email_from
   default reply_to: "noreply@#{Gitlab.config.gitlab.host}"
 
-  # Just send email with 3 seconds delay
+  # Just send email with 2 seconds delay
   def self.delay
     delay_for(2.seconds)
   end

From 44ab156ef45fb1a566395b4d98afe796b28d21c9 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Mon, 17 Feb 2014 16:45:28 +0100
Subject: [PATCH 2/5] Remove the email subject prefix
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This changes email subjects from:

    GitLab | Team / Project | Note for issue #1234

to:

    Team / Project | Note for issue #1234

Rationale:

* Emails should be as meaningful as possible, and emphasize content over
chrome. The "GitLab" name is more chrome than content.

* Users can tell an email coming from GitLab by the sender or the header
in the email content.

* An organization that works mainly with GitLab knows that
every SVC email comes from GitLab. For these organizations, having
"GitLab" in front of every email is just noise hiding the meaningful
information.
---
 app/mailers/emails/merge_requests.rb |  2 +-
 app/mailers/notify.rb                | 12 ++++++------
 spec/mailers/notify_spec.rb          |  6 +++---
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb
index e60887d525..f0b3aab54e 100644
--- a/app/mailers/emails/merge_requests.rb
+++ b/app/mailers/emails/merge_requests.rb
@@ -57,7 +57,7 @@ module Emails
   #   >> subject('Lorem ipsum', 'Dolor sit amet')
   #   => "GitLab Merge Request | Lorem ipsum | Dolor sit amet"
   def subject(*extra)
-    subject = "GitLab Merge Request |"
+    subject = "Merge Request | "
     if @merge_request.for_fork?
       subject << "#{@merge_request.source_project.name_with_namespace}:#{merge_request.source_branch} >> #{@merge_request.target_project.name_with_namespace}:#{merge_request.target_branch}"
     else
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index e58db8ef35..666cef4d3c 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -43,21 +43,21 @@ class Notify < ActionMailer::Base
   # Examples
   #
   #   >> subject('Lorem ipsum')
-  #   => "GitLab | Lorem ipsum"
+  #   => "Lorem ipsum"
   #
   #   # Automatically inserts Project name when @project is set
   #   >> @project = Project.last
   #   => #<Project id: 1, name: "Ruby on Rails", path: "ruby_on_rails", ...>
   #   >> subject('Lorem ipsum')
-  #   => "GitLab | Ruby on Rails | Lorem ipsum "
+  #   => "Ruby on Rails | Lorem ipsum "
   #
   #   # Accepts multiple arguments
   #   >> subject('Lorem ipsum', 'Dolor sit amet')
-  #   => "GitLab | Lorem ipsum | Dolor sit amet"
+  #   => "Lorem ipsum | Dolor sit amet"
   def subject(*extra)
-    subject = "GitLab"
-    subject << (@project ? " | #{@project.name_with_namespace}" : "")
-    subject << " | " + extra.join(' | ') if extra.present?
+    subject = ""
+    subject << "#{@project.name_with_namespace} | " if @project
+    subject << extra.join(' | ') if extra.present?
     subject
   end
 end
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 88cae0bb75..fae726b965 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -24,7 +24,7 @@ describe Notify do
     end
 
     it 'has the correct subject' do
-      should have_subject /^gitlab \| Account was created for you$/i
+      should have_subject /^Account was created for you$/i
     end
 
     it 'contains the new user\'s login name' do
@@ -52,7 +52,7 @@ describe Notify do
     end
 
     it 'has the correct subject' do
-      should have_subject /^gitlab \| Account was created for you$/i
+      should have_subject /^Account was created for you$/i
     end
 
     it 'contains the new user\'s login name' do
@@ -78,7 +78,7 @@ describe Notify do
     end
 
     it 'has the correct subject' do
-      should have_subject /^gitlab \| SSH key was added to your account$/i
+      should have_subject /^SSH key was added to your account$/i
     end
 
     it 'contains the new ssh key title' do

From d8a24003cbd66dc80393dae46299c5734d50b5e0 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Tue, 18 Feb 2014 10:34:57 +0100
Subject: [PATCH 3/5] Don't write the project namespace in email subjects

For instance, the email "Subject" field changes from:

    Mattt / Ground Control | Saving issue doesn't work sometimes (#1234)

to:

    Ground Control | Saving issue doesn't work sometimes (#1234)

Rationale:

* Most people are receiving email notifications about a single fork:
the project activity, issues and merge requests are happening either
in a root repository or in a fork, but less often in several forks.

* It removes noise from the email subject. The namespace of the project
can still be read in the email body.

* For Entreprise users that have a single namespace "MyEntreprise",
having this repeated in every email subject is very noisy.
---
 app/mailers/notify.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 666cef4d3c..3a4c9cf73b 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -56,7 +56,7 @@ class Notify < ActionMailer::Base
   #   => "Lorem ipsum | Dolor sit amet"
   def subject(*extra)
     subject = ""
-    subject << "#{@project.name_with_namespace} | " if @project
+    subject << "#{@project.name} | " if @project
     subject << extra.join(' | ') if extra.present?
     subject
   end

From 57cb1ca7917f27f2c7500858fb66d53b3ea49783 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Mon, 17 Feb 2014 17:08:13 +0100
Subject: [PATCH 4/5] Write the issue/merge request title in the notification
 email subject

This changes the email subjects for issues and merge request
notifications from:

    Team / Project | Note for issue #1234

to:

    Team / Project | Saving issue doesn't work sometimes (#1234)

Rationale:

* Scan the subject of the email notification more easily when catching
up with a lot of notifications. Instead of having to open the email to
get the title of the issue or merge request, one can simply read the
subject of the email.

* Group messages by subject: email clients will group emails in threads
if they have the same subject.
---
 app/mailers/emails/issues.rb         | 10 ++++++----
 app/mailers/emails/merge_requests.rb | 12 ++++++++----
 app/mailers/emails/notes.rb          | 12 ++++++++----
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb
index b2b4b83d6c..dece5112c9 100644
--- a/app/mailers/emails/issues.rb
+++ b/app/mailers/emails/issues.rb
@@ -3,14 +3,16 @@ module Emails
     def new_issue_email(recipient_id, issue_id)
       @issue = Issue.find(issue_id)
       @project = @issue.project
-      mail(to: recipient(recipient_id), subject: subject("New issue ##{@issue.iid}", @issue.title))
+      mail(to: recipient(recipient_id),
+           subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
 
     def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id)
       @issue = Issue.find(issue_id)
       @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
       @project = @issue.project
-      mail(to: recipient(recipient_id), subject: subject("Changed issue ##{@issue.iid}", @issue.title))
+      mail(to: recipient(recipient_id),
+           subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
 
     def closed_issue_email(recipient_id, issue_id, updated_by_user_id)
@@ -18,7 +20,7 @@ module Emails
       @project = @issue.project
       @updated_by = User.find updated_by_user_id
       mail(to: recipient(recipient_id),
-           subject: subject("Closed issue ##{@issue.iid}", @issue.title))
+           subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
 
     def issue_status_changed_email(recipient_id, issue_id, status, updated_by_user_id)
@@ -27,7 +29,7 @@ module Emails
       @project = @issue.project
       @updated_by = User.find updated_by_user_id
       mail(to: recipient(recipient_id),
-           subject: subject("Changed issue ##{@issue.iid}", @issue.title))
+           subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
   end
 end
diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb
index f0b3aab54e..39c02ca07c 100644
--- a/app/mailers/emails/merge_requests.rb
+++ b/app/mailers/emails/merge_requests.rb
@@ -3,27 +3,31 @@ module Emails
     def new_merge_request_email(recipient_id, merge_request_id)
       @merge_request = MergeRequest.find(merge_request_id)
       @project = @merge_request.project
-      mail(to: recipient(recipient_id), subject: subject("New merge request ##{@merge_request.iid}", @merge_request.title))
+      mail(to: recipient(recipient_id),
+           subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
     end
 
     def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id)
       @merge_request = MergeRequest.find(merge_request_id)
       @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
       @project = @merge_request.project
-      mail(to: recipient(recipient_id), subject: subject("Changed merge request ##{@merge_request.iid}", @merge_request.title))
+      mail(to: recipient(recipient_id),
+           subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
     end
 
     def closed_merge_request_email(recipient_id, merge_request_id, updated_by_user_id)
       @merge_request = MergeRequest.find(merge_request_id)
       @updated_by = User.find updated_by_user_id
       @project = @merge_request.project
-      mail(to: recipient(recipient_id), subject: subject("Closed merge request ##{@merge_request.iid}", @merge_request.title))
+      mail(to: recipient(recipient_id),
+           subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
     end
 
     def merged_merge_request_email(recipient_id, merge_request_id)
       @merge_request = MergeRequest.find(merge_request_id)
       @project = @merge_request.project
-      mail(to: recipient(recipient_id), subject: subject("Accepted merge request ##{@merge_request.iid}", @merge_request.title))
+      mail(to: recipient(recipient_id),
+           subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
     end
   end
 
diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb
index e967cf6dc7..b727301df5 100644
--- a/app/mailers/emails/notes.rb
+++ b/app/mailers/emails/notes.rb
@@ -4,27 +4,31 @@ module Emails
       @note = Note.find(note_id)
       @commit = @note.noteable
       @project = @note.project
-      mail(to: recipient(recipient_id), subject: subject("Note for commit #{@commit.short_id}", @commit.title))
+      mail(to: recipient(recipient_id),
+           subject: subject("#{@commit.title} (#{@commit.short_id})"))
     end
 
     def note_issue_email(recipient_id, note_id)
       @note = Note.find(note_id)
       @issue = @note.noteable
       @project = @note.project
-      mail(to: recipient(recipient_id), subject: subject("Note for issue ##{@issue.iid}"))
+      mail(to: recipient(recipient_id),
+           subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
 
     def note_merge_request_email(recipient_id, note_id)
       @note = Note.find(note_id)
       @merge_request = @note.noteable
       @project = @note.project
-      mail(to: recipient(recipient_id), subject: subject("Note for merge request ##{@merge_request.iid}"))
+      mail(to: recipient(recipient_id),
+           subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
     end
 
     def note_wall_email(recipient_id, note_id)
       @note = Note.find(note_id)
       @project = @note.project
-      mail(to: recipient(recipient_id), subject: subject("Note on wall"))
+      mail(to: recipient(recipient_id),
+           subject: subject("Note on wall"))
     end
   end
 end

From 96dded3ec8401e9832b3888338f37c846bd43583 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Mon, 17 Feb 2014 18:49:42 +0100
Subject: [PATCH 5/5] Send emails from the author

This changes the email "From" field from "gitlab@example.com" to either:

* "John Doe <gitlab@example.com>" if the author of the action is known,
* "GitLab <gitlab@example.com>" otherwise.

Rationale: this allow mails to appear as if they were sent by the
author. It appears in the mailbox more like a real discussion between
the sender and the receiver ("John sent: we should refactor this") and
less like a robot notifying about something.
---
 app/mailers/emails/issues.rb               | 14 ++--
 app/mailers/emails/merge_requests.rb       | 14 ++--
 app/mailers/emails/notes.rb                | 12 ++-
 app/mailers/emails/projects.rb             |  4 +-
 app/mailers/notify.rb                      | 19 ++++-
 app/services/notification_service.rb       |  2 +-
 config/initializers/devise.rb              |  2 +-
 spec/mailers/notify_spec.rb                | 90 ++++++++++++++++++----
 spec/services/notification_service_spec.rb |  8 +-
 9 files changed, 126 insertions(+), 39 deletions(-)

diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb
index dece5112c9..3adb47dc5b 100644
--- a/app/mailers/emails/issues.rb
+++ b/app/mailers/emails/issues.rb
@@ -3,15 +3,17 @@ module Emails
     def new_issue_email(recipient_id, issue_id)
       @issue = Issue.find(issue_id)
       @project = @issue.project
-      mail(to: recipient(recipient_id),
+      mail(from: sender(@issue.author_id),
+           to: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
 
-    def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id)
+    def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id, updated_by_user_id)
       @issue = Issue.find(issue_id)
       @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
       @project = @issue.project
-      mail(to: recipient(recipient_id),
+      mail(from: sender(updated_by_user_id),
+           to: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
 
@@ -19,7 +21,8 @@ module Emails
       @issue = Issue.find issue_id
       @project = @issue.project
       @updated_by = User.find updated_by_user_id
-      mail(to: recipient(recipient_id),
+      mail(from: sender(updated_by_user_id),
+           to: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
 
@@ -28,7 +31,8 @@ module Emails
       @issue_status = status
       @project = @issue.project
       @updated_by = User.find updated_by_user_id
-      mail(to: recipient(recipient_id),
+      mail(from: sender(updated_by_user_id),
+           to: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
   end
diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb
index 39c02ca07c..0845e14edc 100644
--- a/app/mailers/emails/merge_requests.rb
+++ b/app/mailers/emails/merge_requests.rb
@@ -3,15 +3,17 @@ module Emails
     def new_merge_request_email(recipient_id, merge_request_id)
       @merge_request = MergeRequest.find(merge_request_id)
       @project = @merge_request.project
-      mail(to: recipient(recipient_id),
+      mail(from: sender(@merge_request.author_id),
+           to: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
     end
 
-    def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id)
+    def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id, updated_by_user_id)
       @merge_request = MergeRequest.find(merge_request_id)
       @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
       @project = @merge_request.project
-      mail(to: recipient(recipient_id),
+      mail(from: sender(updated_by_user_id),
+           to: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
     end
 
@@ -19,14 +21,16 @@ module Emails
       @merge_request = MergeRequest.find(merge_request_id)
       @updated_by = User.find updated_by_user_id
       @project = @merge_request.project
-      mail(to: recipient(recipient_id),
+      mail(from: sender(updated_by_user_id),
+           to: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
     end
 
     def merged_merge_request_email(recipient_id, merge_request_id)
       @merge_request = MergeRequest.find(merge_request_id)
       @project = @merge_request.project
-      mail(to: recipient(recipient_id),
+      mail(from: sender(@merge_request.author_id_of_changes),
+           to: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
     end
   end
diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb
index b727301df5..00b127da42 100644
--- a/app/mailers/emails/notes.rb
+++ b/app/mailers/emails/notes.rb
@@ -4,7 +4,8 @@ module Emails
       @note = Note.find(note_id)
       @commit = @note.noteable
       @project = @note.project
-      mail(to: recipient(recipient_id),
+      mail(from: sender(@note.author_id),
+           to: recipient(recipient_id),
            subject: subject("#{@commit.title} (#{@commit.short_id})"))
     end
 
@@ -12,7 +13,8 @@ module Emails
       @note = Note.find(note_id)
       @issue = @note.noteable
       @project = @note.project
-      mail(to: recipient(recipient_id),
+      mail(from: sender(@note.author_id),
+           to: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
     end
 
@@ -20,14 +22,16 @@ module Emails
       @note = Note.find(note_id)
       @merge_request = @note.noteable
       @project = @note.project
-      mail(to: recipient(recipient_id),
+      mail(from: sender(@note.author_id),
+           to: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
     end
 
     def note_wall_email(recipient_id, note_id)
       @note = Note.find(note_id)
       @project = @note.project
-      mail(to: recipient(recipient_id),
+      mail(from: sender(@note.author_id),
+           to: recipient(recipient_id),
            subject: subject("Note on wall"))
     end
   end
diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb
index 428d74d83c..46f24e9fb7 100644
--- a/app/mailers/emails/projects.rb
+++ b/app/mailers/emails/projects.rb
@@ -22,7 +22,9 @@ module Emails
       @diffs   = compare.diffs
       @branch  = branch
 
-      mail(to: recipient, subject: subject("New push to repository"))
+      mail(from: sender(author_id),
+           to: recipient,
+           subject: subject("New push to repository"))
     end
   end
 end
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 3a4c9cf73b..554f53cf14 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -15,7 +15,7 @@ class Notify < ActionMailer::Base
   default_url_options[:port]     = Gitlab.config.gitlab.port unless Gitlab.config.gitlab_on_standard_port?
   default_url_options[:script_name] = Gitlab.config.gitlab.relative_url_root
 
-  default from: Gitlab.config.gitlab.email_from
+  default from: Proc.new { default_sender_address.format }
   default reply_to: "noreply@#{Gitlab.config.gitlab.host}"
 
   # Just send email with 2 seconds delay
@@ -25,6 +25,23 @@ class Notify < ActionMailer::Base
 
   private
 
+  # The default email address to send emails from
+  def default_sender_address
+    address = Mail::Address.new(Gitlab.config.gitlab.email_from)
+    address.display_name = "GitLab"
+    address
+  end
+
+  # 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)
+    if sender = User.find(sender_id)
+      address = default_sender_address
+      address.display_name = sender.name
+      address.format
+    end
+  end
+
   # Look up a User by their ID and return their email address
   #
   # recipient_id - User ID
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 9d7bb9639a..5daf573630 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -257,7 +257,7 @@ class NotificationService
     recipients.delete(current_user)
 
     recipients.each do |recipient|
-      mailer.send(method, recipient.id, target.id, target.assignee_id_was)
+      mailer.send(method, recipient.id, target.id, target.assignee_id_was, current_user.id)
     end
   end
 
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
index a02bf9d4ae..50669ece7a 100644
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -4,7 +4,7 @@ Devise.setup do |config|
   # ==> Mailer Configuration
   # Configure the e-mail address which will be shown in Devise::Mailer,
   # note that it will be overwritten if you use your own mailer class with default "from" parameter.
-  config.mailer_sender = Gitlab.config.gitlab.email_from
+  config.mailer_sender = "GitLab <#{Gitlab.config.gitlab.email_from}>"
 
 
   # Configure the class responsible to send e-mails.
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index fae726b965..26b7ca876f 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -4,6 +4,7 @@ describe Notify do
   include EmailSpec::Helpers
   include EmailSpec::Matchers
 
+  let(:gitlab_sender) { Gitlab.config.gitlab.email_from }
   let(:recipient) { create(:user, email: 'recipient@example.com') }
   let(:project) { create(:project) }
 
@@ -13,12 +14,22 @@ describe Notify do
     end
   end
 
+  shared_examples 'an email sent from GitLab' do
+    it 'is sent from GitLab' do
+      sender = subject.header[:from].addrs[0]
+      sender.display_name.should eq('GitLab')
+      sender.address.should eq(gitlab_sender)
+    end
+  end
+
   describe 'for new users, the email' do
     let(:example_site_path) { root_path }
     let(:new_user) { create(:user, email: 'newguy@example.com', created_by_id: 1) }
 
     subject { Notify.new_user_email(new_user.id, new_user.password) }
 
+    it_behaves_like 'an email sent from GitLab'
+
     it 'is sent to the new user' do
       should deliver_to new_user.email
     end
@@ -47,6 +58,8 @@ describe Notify do
 
     subject { Notify.new_user_email(new_user.id, new_user.password) }
 
+    it_behaves_like 'an email sent from GitLab'
+
     it 'is sent to the new user' do
       should deliver_to new_user.email
     end
@@ -73,6 +86,8 @@ describe Notify do
 
     subject { Notify.new_ssh_key_email(key.id) }
 
+    it_behaves_like 'an email sent from GitLab'
+
     it 'is sent to the new user' do
       should deliver_to key.user.email
     end
@@ -114,17 +129,24 @@ describe Notify do
 
   context 'for a project' do
     describe 'items that are assignable, the email' do
+      let(:current_user) { create(:user, email: "current@email.com") }
       let(:assignee) { create(:user, email: 'assignee@example.com') }
       let(:previous_assignee) { create(:user, name: 'Previous Assignee') }
 
       shared_examples 'an assignee email' do
+        it 'is sent as the author' do
+          sender = subject.header[:from].addrs[0]
+          sender.display_name.should eq(current_user.name)
+          sender.address.should eq(gitlab_sender)
+        end
+
         it 'is sent to the assignee' do
           should deliver_to assignee.email
         end
       end
 
       context 'for issues' do
-        let(:issue) { create(:issue, assignee: assignee, project: project ) }
+        let(:issue) { create(:issue, author: current_user, assignee: assignee, project: project ) }
 
         describe 'that are new' do
           subject { Notify.new_issue_email(issue.assignee_id, issue.id) }
@@ -132,7 +154,7 @@ describe Notify do
           it_behaves_like 'an assignee email'
 
           it 'has the correct subject' do
-            should have_subject /#{project.name} \| New issue ##{issue.iid} \| #{issue.title}/
+            should have_subject /#{project.name} \| #{issue.title} \(##{issue.iid}\)/
           end
 
           it 'contains a link to the new issue' do
@@ -141,14 +163,18 @@ describe Notify do
         end
 
         describe 'that have been reassigned' do
-          before(:each) { issue.stub(:assignee_id_was).and_return(previous_assignee.id) }
-
-          subject { Notify.reassigned_issue_email(recipient.id, issue.id, previous_assignee.id) }
+          subject { Notify.reassigned_issue_email(recipient.id, issue.id, previous_assignee.id, current_user) }
 
           it_behaves_like 'a multiple recipients email'
 
+          it 'is sent as the author' do
+            sender = subject.header[:from].addrs[0]
+            sender.display_name.should eq(current_user.name)
+            sender.address.should eq(gitlab_sender)
+          end
+
           it 'has the correct subject' do
-            should have_subject /Changed issue ##{issue.iid} \| #{issue.title}/
+            should have_subject /#{issue.title} \(##{issue.iid}\)/
           end
 
           it 'contains the name of the previous assignee' do
@@ -165,12 +191,17 @@ describe Notify do
         end
 
         describe 'status changed' do
-          let(:current_user) { create(:user, email: "current@email.com") }
           let(:status) { 'closed' }
           subject { Notify.issue_status_changed_email(recipient.id, issue.id, status, current_user) }
 
+          it 'is sent as the author' do
+            sender = subject.header[:from].addrs[0]
+            sender.display_name.should eq(current_user.name)
+            sender.address.should eq(gitlab_sender)
+          end
+
           it 'has the correct subject' do
-            should have_subject /Changed issue ##{issue.iid} \| #{issue.title}/i
+            should have_subject /#{issue.title} \(##{issue.iid}\)/i
           end
 
           it 'contains the new status' do
@@ -189,7 +220,7 @@ describe Notify do
       end
 
       context 'for merge requests' do
-        let(:merge_request) { create(:merge_request, assignee: assignee, source_project: project, target_project: project) }
+        let(:merge_request) { create(:merge_request, author: current_user, assignee: assignee, source_project: project, target_project: project) }
 
         describe 'that are new' do
           subject { Notify.new_merge_request_email(merge_request.assignee_id, merge_request.id) }
@@ -197,7 +228,7 @@ describe Notify do
           it_behaves_like 'an assignee email'
 
           it 'has the correct subject' do
-            should have_subject /New merge request ##{merge_request.iid}/
+            should have_subject /#{merge_request.title} \(!#{merge_request.iid}\)/
           end
 
           it 'contains a link to the new merge request' do
@@ -214,14 +245,18 @@ describe Notify do
         end
 
         describe 'that are reassigned' do
-          before(:each) { merge_request.stub(:assignee_id_was).and_return(previous_assignee.id) }
-
-          subject { Notify.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id) }
+          subject { Notify.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) }
 
           it_behaves_like 'a multiple recipients email'
 
+          it 'is sent as the author' do
+            sender = subject.header[:from].addrs[0]
+            sender.display_name.should eq(current_user.name)
+            sender.address.should eq(gitlab_sender)
+          end
+
           it 'has the correct subject' do
-            should have_subject /Changed merge request ##{merge_request.iid}/
+            should have_subject /#{merge_request.title} \(!#{merge_request.iid}\)/
           end
 
           it 'contains the name of the previous assignee' do
@@ -245,6 +280,8 @@ describe Notify do
       let(:user) { create(:user) }
       subject { Notify.project_was_moved_email(project.id, user.id) }
 
+      it_behaves_like 'an email sent from GitLab'
+
       it 'has the correct subject' do
         should have_subject /Project was moved/
       end
@@ -265,6 +302,9 @@ describe Notify do
                                    project: project,
                                    user: user) }
       subject { Notify.project_access_granted_email(users_project.id) }
+
+      it_behaves_like 'an email sent from GitLab'
+
       it 'has the correct subject' do
         should have_subject /Access to project was granted/
       end
@@ -285,6 +325,12 @@ describe Notify do
       end
 
       shared_examples 'a note email' do
+        it 'is sent as the author' do
+          sender = subject.header[:from].addrs[0]
+          sender.display_name.should eq(note_author.name)
+          sender.address.should eq(gitlab_sender)
+        end
+
         it 'is sent to the given recipient' do
           should deliver_to recipient.email
         end
@@ -324,7 +370,7 @@ describe Notify do
         it_behaves_like 'a note email'
 
         it 'has the correct subject' do
-          should have_subject /Note for commit #{commit.short_id}/
+          should have_subject /#{commit.title} \(#{commit.short_id}\)/
         end
 
         it 'contains a link to the commit' do
@@ -342,7 +388,7 @@ describe Notify do
         it_behaves_like 'a note email'
 
         it 'has the correct subject' do
-          should have_subject /Note for merge request ##{merge_request.iid}/
+          should have_subject /#{merge_request.title} \(!#{merge_request.iid}\)/
         end
 
         it 'contains a link to the merge request note' do
@@ -360,7 +406,7 @@ describe Notify do
         it_behaves_like 'a note email'
 
         it 'has the correct subject' do
-          should have_subject /Note for issue ##{issue.iid}/
+          should have_subject /#{issue.title} \(##{issue.iid}\)/
         end
 
         it 'contains a link to the issue note' do
@@ -377,6 +423,8 @@ describe Notify do
 
     subject { Notify.group_access_granted_email(membership.id) }
 
+    it_behaves_like 'an email sent from GitLab'
+
     it 'has the correct subject' do
       should have_subject /Access to group was granted/
     end
@@ -401,6 +449,8 @@ describe Notify do
 
     subject { ActionMailer::Base.deliveries.last }
 
+    it_behaves_like 'an email sent from GitLab'
+
     it 'is sent to the new user' do
       should deliver_to 'new-email@mail.com'
     end
@@ -421,6 +471,12 @@ describe Notify do
 
     subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'master', compare) }
 
+    it 'is sent as the author' do
+      sender = subject.header[:from].addrs[0]
+      sender.display_name.should eq(user.name)
+      sender.address.should eq(gitlab_sender)
+    end
+
     it 'is sent to recipient' do
       should deliver_to 'devs@company.name'
     end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index e378be0425..077ad8b6e1 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -137,11 +137,11 @@ describe NotificationService do
       end
 
       def should_email(user_id)
-        Notify.should_receive(:reassigned_issue_email).with(user_id, issue.id, issue.assignee_id)
+        Notify.should_receive(:reassigned_issue_email).with(user_id, issue.id, issue.assignee_id, @u_disabled.id)
       end
 
       def should_not_email(user_id)
-        Notify.should_not_receive(:reassigned_issue_email).with(user_id, issue.id, issue.assignee_id)
+        Notify.should_not_receive(:reassigned_issue_email).with(user_id, issue.id, issue.assignee_id, @u_disabled.id)
       end
     end
 
@@ -201,11 +201,11 @@ describe NotificationService do
       end
 
       def should_email(user_id)
-        Notify.should_receive(:reassigned_merge_request_email).with(user_id, merge_request.id, merge_request.assignee_id)
+        Notify.should_receive(:reassigned_merge_request_email).with(user_id, merge_request.id, merge_request.assignee_id, merge_request.author_id)
       end
 
       def should_not_email(user_id)
-        Notify.should_not_receive(:reassigned_merge_request_email).with(user_id, merge_request.id, merge_request.assignee_id)
+        Notify.should_not_receive(:reassigned_merge_request_email).with(user_id, merge_request.id, merge_request.assignee_id, merge_request.author_id)
       end
     end