From 7ba0b502d3e6fdc5efc5619d803a1b9fcf9db791 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Tue, 18 Feb 2014 11:12:01 +0100
Subject: [PATCH 1/9] Add a "View in GitLab" link in notification emails

When an email notification concerns a specific object (issue, note,
merge request, etc.), add a link to the footer of the email that opens
the item's page in a web browser.

Rationale:

* The link is predictable: always the same text, always at the same
location, like any reliable tool.

* It allows to remove the inline-title in many emails, and leave only
the actual content of the message.
---
 app/mailers/emails/groups.rb         | 2 +-
 app/mailers/emails/issues.rb         | 4 ++++
 app/mailers/emails/merge_requests.rb | 4 ++++
 app/mailers/emails/notes.rb          | 4 ++++
 app/mailers/emails/profile.rb        | 2 ++
 app/mailers/emails/projects.rb       | 7 +++++++
 app/views/layouts/notify.html.haml   | 3 +++
 spec/mailers/notify_spec.rb          | 6 ++++++
 8 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/app/mailers/emails/groups.rb b/app/mailers/emails/groups.rb
index 1c8ae122c4..1654fc55bc 100644
--- a/app/mailers/emails/groups.rb
+++ b/app/mailers/emails/groups.rb
@@ -3,7 +3,7 @@ module Emails
     def group_access_granted_email(user_group_id)
       @membership = UsersGroup.find(user_group_id)
       @group = @membership.group
-
+      @target_url = group_url(@group)
       mail(to: @membership.user.email,
            subject: subject("Access to group was granted"))
     end
diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb
index 3adb47dc5b..d684e35445 100644
--- a/app/mailers/emails/issues.rb
+++ b/app/mailers/emails/issues.rb
@@ -3,6 +3,7 @@ module Emails
     def new_issue_email(recipient_id, issue_id)
       @issue = Issue.find(issue_id)
       @project = @issue.project
+      @target_url = project_issue_url(@project, @issue)
       mail(from: sender(@issue.author_id),
            to: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
@@ -12,6 +13,7 @@ module Emails
       @issue = Issue.find(issue_id)
       @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
       @project = @issue.project
+      @target_url = project_issue_url(@project, @issue)
       mail(from: sender(updated_by_user_id),
            to: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
@@ -21,6 +23,7 @@ module Emails
       @issue = Issue.find issue_id
       @project = @issue.project
       @updated_by = User.find updated_by_user_id
+      @target_url = project_issue_url(@project, @issue)
       mail(from: sender(updated_by_user_id),
            to: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
@@ -31,6 +34,7 @@ module Emails
       @issue_status = status
       @project = @issue.project
       @updated_by = User.find updated_by_user_id
+      @target_url = project_issue_url(@project, @issue)
       mail(from: sender(updated_by_user_id),
            to: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb
index 0845e14edc..5e1b8faf13 100644
--- a/app/mailers/emails/merge_requests.rb
+++ b/app/mailers/emails/merge_requests.rb
@@ -3,6 +3,7 @@ module Emails
     def new_merge_request_email(recipient_id, merge_request_id)
       @merge_request = MergeRequest.find(merge_request_id)
       @project = @merge_request.project
+      @target_url = project_merge_request_url(@project, @merge_request)
       mail(from: sender(@merge_request.author_id),
            to: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
@@ -12,6 +13,7 @@ module Emails
       @merge_request = MergeRequest.find(merge_request_id)
       @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
       @project = @merge_request.project
+      @target_url = project_merge_request_url(@project, @merge_request)
       mail(from: sender(updated_by_user_id),
            to: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
@@ -21,6 +23,7 @@ module Emails
       @merge_request = MergeRequest.find(merge_request_id)
       @updated_by = User.find updated_by_user_id
       @project = @merge_request.project
+      @target_url = project_merge_request_url(@project, @merge_request)
       mail(from: sender(updated_by_user_id),
            to: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
@@ -29,6 +32,7 @@ module Emails
     def merged_merge_request_email(recipient_id, merge_request_id)
       @merge_request = MergeRequest.find(merge_request_id)
       @project = @merge_request.project
+      @target_url = project_merge_request_url(@project, @merge_request)
       mail(from: sender(@merge_request.author_id_of_changes),
            to: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb
index 00b127da42..22c4ededf5 100644
--- a/app/mailers/emails/notes.rb
+++ b/app/mailers/emails/notes.rb
@@ -4,6 +4,7 @@ module Emails
       @note = Note.find(note_id)
       @commit = @note.noteable
       @project = @note.project
+      @target_url = project_commit_url(@project, @commit, anchor: "note_#{@note.id}")
       mail(from: sender(@note.author_id),
            to: recipient(recipient_id),
            subject: subject("#{@commit.title} (#{@commit.short_id})"))
@@ -13,6 +14,7 @@ module Emails
       @note = Note.find(note_id)
       @issue = @note.noteable
       @project = @note.project
+      @target_url = project_issue_url(@project, @issue)
       mail(from: sender(@note.author_id),
            to: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
@@ -22,6 +24,7 @@ module Emails
       @note = Note.find(note_id)
       @merge_request = @note.noteable
       @project = @note.project
+      @target_url = project_merge_request_url(@project, @merge_request, anchor: "note_#{@note.id}")
       mail(from: sender(@note.author_id),
            to: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
@@ -30,6 +33,7 @@ module Emails
     def note_wall_email(recipient_id, note_id)
       @note = Note.find(note_id)
       @project = @note.project
+      @target_url = project_wall_url(@note.project, anchor: "note_#{@note.id}")
       mail(from: sender(@note.author_id),
            to: recipient(recipient_id),
            subject: subject("Note on wall"))
diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb
index c91660a02b..f02d95fd55 100644
--- a/app/mailers/emails/profile.rb
+++ b/app/mailers/emails/profile.rb
@@ -3,6 +3,7 @@ module Emails
     def new_user_email(user_id, password)
       @user = User.find(user_id)
       @password = password
+      @target_url = user_url(@user)
       mail(to: @user.email, subject: subject("Account was created for you"))
     end
 
@@ -15,6 +16,7 @@ module Emails
     def new_ssh_key_email(key_id)
       @key = Key.find(key_id)
       @user = @key.user
+      @target_url = user_url(@user)
       mail(to: @user.email, subject: subject("SSH key was added to your account"))
     end
   end
diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb
index 46f24e9fb7..46aa34d13d 100644
--- a/app/mailers/emails/projects.rb
+++ b/app/mailers/emails/projects.rb
@@ -3,6 +3,7 @@ module Emails
     def project_access_granted_email(user_project_id)
       @users_project = UsersProject.find user_project_id
       @project = @users_project.project
+      @target_url = project_url(@project)
       mail(to: @users_project.user.email,
            subject: subject("Access to project was granted"))
     end
@@ -10,6 +11,7 @@ module Emails
     def project_was_moved_email(project_id, user_id)
       @user = User.find user_id
       @project = Project.find project_id
+      @target_url = project_url(@project)
       mail(to: @user.email,
            subject: subject("Project was moved"))
     end
@@ -21,6 +23,11 @@ module Emails
       @commits = Commit.decorate(compare.commits)
       @diffs   = compare.diffs
       @branch  = branch
+      if @commits.length > 1
+        @target_url = project_compare_url(@project, from: @commits.first, to: @commits.last)
+      else
+        @target_url = project_commit_url(@project, @compare.commit)
+      end
 
       mail(from: sender(author_id),
            to: recipient,
diff --git a/app/views/layouts/notify.html.haml b/app/views/layouts/notify.html.haml
index f88abeca88..2a16710524 100644
--- a/app/views/layouts/notify.html.haml
+++ b/app/views/layouts/notify.html.haml
@@ -20,3 +20,6 @@
           %p{style: "font-size:small;color:#777"}
             - if @project
               You're receiving this notification because you are a member of the #{@project.name_with_namespace} project team.
+              %br
+            - if @target_url
+              #{link_to "View in GitLab", @target_url}
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 6ba4d97ad4..6b8d7a6f04 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -468,6 +468,8 @@ describe Notify do
     let(:example_site_path) { root_path }
     let(:user) { create(:user) }
     let(:compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, 'cd5c4bac', 'b1e6a9db') }
+    let(:commits) { Commit.decorate(compare.commits) }
+    let(:diff_path) { project_compare_path(project, from: commits.first, to: commits.last) }
 
     subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'master', compare) }
 
@@ -492,5 +494,9 @@ describe Notify do
     it 'includes diffs' do
       should have_body_text /Checkout wiki pages for installation information/
     end
+
+    it 'contains a link to the diff' do
+      should have_body_text /#{diff_path}/
+    end
   end
 end

From edd6028bdb1f0e8ba6744540e9d3f324ab40126d Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Tue, 18 Feb 2014 11:46:01 +0100
Subject: [PATCH 2/9] Remove custom link to target in notification emails

Every email has a different way of showing a link to the discussion on
the website. We don't need this anymore, as we now have a standard
"View in GitLab" link in the footer of every email.
---
 app/mailers/emails/notes.rb                               | 2 +-
 app/views/notify/closed_issue_email.html.haml             | 3 ---
 app/views/notify/closed_merge_request_email.html.haml     | 2 --
 app/views/notify/group_access_granted_email.html.haml     | 3 ---
 app/views/notify/issue_status_changed_email.html.haml     | 3 ---
 app/views/notify/merged_merge_request_email.html.haml     | 2 --
 app/views/notify/new_issue_email.html.haml                | 3 ---
 app/views/notify/new_merge_request_email.html.haml        | 2 --
 app/views/notify/note_commit_email.html.haml              | 3 ---
 app/views/notify/note_issue_email.html.haml               | 3 ---
 app/views/notify/note_merge_request_email.html.haml       | 1 -
 app/views/notify/note_wall_email.html.haml                | 4 ----
 app/views/notify/reassigned_issue_email.html.haml         | 3 ---
 app/views/notify/reassigned_merge_request_email.html.haml | 3 ---
 14 files changed, 1 insertion(+), 36 deletions(-)

diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb
index 22c4ededf5..ccbdadf010 100644
--- a/app/mailers/emails/notes.rb
+++ b/app/mailers/emails/notes.rb
@@ -14,7 +14,7 @@ module Emails
       @note = Note.find(note_id)
       @issue = @note.noteable
       @project = @note.project
-      @target_url = project_issue_url(@project, @issue)
+      @target_url = project_issue_url(@project, @issue, anchor: "note_#{@note.id}")
       mail(from: sender(@note.author_id),
            to: recipient(recipient_id),
            subject: subject("#{@issue.title} (##{@issue.iid})"))
diff --git a/app/views/notify/closed_issue_email.html.haml b/app/views/notify/closed_issue_email.html.haml
index 325cd44eb4..56c18cd83c 100644
--- a/app/views/notify/closed_issue_email.html.haml
+++ b/app/views/notify/closed_issue_email.html.haml
@@ -1,5 +1,2 @@
 %p
   = "Issue was closed by #{@updated_by.name}"
-%p
-  = "Issue ##{@issue.iid}"
-  = link_to_gfm truncate(@issue.title, length: 45), project_issue_url(@issue.project, @issue), title: @issue.title
diff --git a/app/views/notify/closed_merge_request_email.html.haml b/app/views/notify/closed_merge_request_email.html.haml
index 45770cc85d..3157f59b2b 100644
--- a/app/views/notify/closed_merge_request_email.html.haml
+++ b/app/views/notify/closed_merge_request_email.html.haml
@@ -1,7 +1,5 @@
 %p
   = "Merge Request #{@merge_request.iid} was closed by #{@updated_by.name}"
-%p
-  = link_to_gfm truncate(@merge_request.title, length: 40), project_merge_request_url(@merge_request.target_project, @merge_request)
 %p
   != merge_path_description(@merge_request, '&rarr;')
 %p
diff --git a/app/views/notify/group_access_granted_email.html.haml b/app/views/notify/group_access_granted_email.html.haml
index 5023ec737a..0092a947ee 100644
--- a/app/views/notify/group_access_granted_email.html.haml
+++ b/app/views/notify/group_access_granted_email.html.haml
@@ -1,5 +1,2 @@
 %p
   = "You have been granted #{@membership.human_access} access to group"
-%p
-  = link_to group_url(@group) do
-    = @group.name
diff --git a/app/views/notify/issue_status_changed_email.html.haml b/app/views/notify/issue_status_changed_email.html.haml
index 7706b3f751..482c884a9d 100644
--- a/app/views/notify/issue_status_changed_email.html.haml
+++ b/app/views/notify/issue_status_changed_email.html.haml
@@ -1,5 +1,2 @@
 %p
   = "Issue was #{@issue_status} by #{@updated_by.name}"
-%p
-  = "Issue ##{@issue.iid}"
-  = link_to_gfm truncate(@issue.title, length: 45), project_issue_url(@issue.project, @issue), title: @issue.title
diff --git a/app/views/notify/merged_merge_request_email.html.haml b/app/views/notify/merged_merge_request_email.html.haml
index e2bc9cf5c0..e3a5239fe0 100644
--- a/app/views/notify/merged_merge_request_email.html.haml
+++ b/app/views/notify/merged_merge_request_email.html.haml
@@ -1,7 +1,5 @@
 %p
   = "Merge Request #{@merge_request.iid} was merged"
-%p
-  = link_to_gfm truncate(@merge_request.title, length: 40), project_merge_request_url(@merge_request.target_project, @merge_request)
 %p
   != merge_path_description(@merge_request, '&rarr;')
 %p
diff --git a/app/views/notify/new_issue_email.html.haml b/app/views/notify/new_issue_email.html.haml
index 3b3c148517..05a9142471 100644
--- a/app/views/notify/new_issue_email.html.haml
+++ b/app/views/notify/new_issue_email.html.haml
@@ -1,8 +1,5 @@
 %p
   New Issue was created.
-%p
-  = "Issue ##{@issue.iid}"
-  = link_to_gfm truncate(@issue.title, length: 45), project_issue_url(@issue.project, @issue), title: @issue.title
 %p
   Author: #{@issue.author_name}
 %p
diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml
index 321f9418de..bc52824c44 100644
--- a/app/views/notify/new_merge_request_email.html.haml
+++ b/app/views/notify/new_merge_request_email.html.haml
@@ -1,7 +1,5 @@
 %p
   = "New Merge Request ##{@merge_request.iid}"
-%p
-  = link_to_gfm truncate(@merge_request.title, length: 40), project_merge_request_url(@merge_request.target_project, @merge_request)
 %p
   != merge_path_description(@merge_request, '&rarr;')
 %p
diff --git a/app/views/notify/note_commit_email.html.haml b/app/views/notify/note_commit_email.html.haml
index 620b258fc1..1d961e4424 100644
--- a/app/views/notify/note_commit_email.html.haml
+++ b/app/views/notify/note_commit_email.html.haml
@@ -1,5 +1,2 @@
-%p
-  = "New comment for Commit #{@commit.short_id}"
-  = link_to_gfm truncate(@commit.title, length: 16), project_commit_url(@note.project, id: @commit.id, anchor: "note_#{@note.id}")
 = render 'note_message'
 
diff --git a/app/views/notify/note_issue_email.html.haml b/app/views/notify/note_issue_email.html.haml
index b3230953e7..2fa2f78466 100644
--- a/app/views/notify/note_issue_email.html.haml
+++ b/app/views/notify/note_issue_email.html.haml
@@ -1,4 +1 @@
-%p
-  = "New comment for Issue ##{@issue.iid}"
-  = link_to_gfm truncate(@issue.title, length: 35), project_issue_url(@issue.project, @issue, anchor: "note_#{@note.id}")
 = render 'note_message'
diff --git a/app/views/notify/note_merge_request_email.html.haml b/app/views/notify/note_merge_request_email.html.haml
index d587b06848..83d07aa0c9 100644
--- a/app/views/notify/note_merge_request_email.html.haml
+++ b/app/views/notify/note_merge_request_email.html.haml
@@ -4,5 +4,4 @@
   - else
     = link_to "New comment", project_merge_request_url(@merge_request.target_project, @merge_request, anchor: "note_#{@note.id}")
   for Merge Request ##{@merge_request.iid}
-  %cite "#{truncate(@merge_request.title, length: 20)}"
 = render 'note_message'
diff --git a/app/views/notify/note_wall_email.html.haml b/app/views/notify/note_wall_email.html.haml
index 92200e83ef..2fa2f78466 100644
--- a/app/views/notify/note_wall_email.html.haml
+++ b/app/views/notify/note_wall_email.html.haml
@@ -1,5 +1 @@
-%p
-  New message on
-  = link_to "Project Wall", project_wall_url(@note.project, anchor: "note_#{@note.id}")
-
 = render 'note_message'
diff --git a/app/views/notify/reassigned_issue_email.html.haml b/app/views/notify/reassigned_issue_email.html.haml
index b0edff44ce..07227a3e68 100644
--- a/app/views/notify/reassigned_issue_email.html.haml
+++ b/app/views/notify/reassigned_issue_email.html.haml
@@ -1,6 +1,3 @@
-%p
-  = "Reassigned Issue ##{@issue.iid}"
-  = link_to_gfm truncate(@issue.title, length: 30), project_issue_url(@issue.project, @issue)
 %p
   Assignee changed
   - if @previous_assignee
diff --git a/app/views/notify/reassigned_merge_request_email.html.haml b/app/views/notify/reassigned_merge_request_email.html.haml
index d2d82d36c4..00aee6bc95 100644
--- a/app/views/notify/reassigned_merge_request_email.html.haml
+++ b/app/views/notify/reassigned_merge_request_email.html.haml
@@ -1,6 +1,3 @@
-%p
-  = "Reassigned Merge Request ##{@merge_request.iid}"
-  = link_to_gfm truncate(@merge_request.title, length: 30), project_merge_request_url(@merge_request.target_project, @merge_request)
 %p
   Assignee changed
   - if @previous_assignee

From b59d105c0e541520073761b15ce575be3518cef2 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Tue, 18 Feb 2014 11:13:16 +0100
Subject: [PATCH 3/9] Linkify the reference to the project in the notification
 emails footer

Rationale:

* Consistency: the link is always at the same place.

* Streamlining: it allows to remove the header banner, now that the
project url can be accessed from another place.
---
 app/views/layouts/notify.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/layouts/notify.html.haml b/app/views/layouts/notify.html.haml
index 2a16710524..9aceb58e8d 100644
--- a/app/views/layouts/notify.html.haml
+++ b/app/views/layouts/notify.html.haml
@@ -19,7 +19,7 @@
         %td{align: "left", style: "margin: 0; padding: 10px;"}
           %p{style: "font-size:small;color:#777"}
             - if @project
-              You're receiving this notification because you are a member of the #{@project.name_with_namespace} project team.
+              You're receiving this notification because you are a member of the #{link_to @project.name_with_namespace, project_url(@project)} project team.
               %br
             - if @target_url
               #{link_to "View in GitLab", @target_url}

From 8e421d2bcb8088fa065b9bd8a6e1776c5140f4cd Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Tue, 18 Feb 2014 14:44:00 +0100
Subject: [PATCH 4/9] Add the description to the "new issue" and "new merge
 request" emails

Previously the content of the issue or merge request was missing from
the email.
---
 app/views/notify/new_issue_email.html.haml    | 12 +++++------
 .../notify/new_merge_request_email.html.haml  |  8 ++++++--
 spec/mailers/notify_spec.rb                   | 20 ++++++++++++++++++-
 3 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/app/views/notify/new_issue_email.html.haml b/app/views/notify/new_issue_email.html.haml
index 05a9142471..f2f8eee18c 100644
--- a/app/views/notify/new_issue_email.html.haml
+++ b/app/views/notify/new_issue_email.html.haml
@@ -1,6 +1,6 @@
-%p
-  New Issue was created.
-%p
-  Author: #{@issue.author_name}
-%p
-  Assignee: #{@issue.assignee_name}
+-if @issue.description
+  = markdown(@issue.description)
+
+- if @issue.assignee_id.present?
+  %p
+    Assignee: #{@issue.assignee_name}
diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml
index bc52824c44..2c012c4b21 100644
--- a/app/views/notify/new_merge_request_email.html.haml
+++ b/app/views/notify/new_merge_request_email.html.haml
@@ -2,6 +2,10 @@
   = "New Merge Request ##{@merge_request.iid}"
 %p
   != merge_path_description(@merge_request, '&rarr;')
-%p
-  Assignee: #{@merge_request.author_name} &rarr; #{@merge_request.assignee_name}
 
+- if @merge_request.assignee_id.present?
+  %p
+    Assignee: #{@merge_request.author_name} &rarr; #{@merge_request.assignee_name}
+
+-if @merge_request.description
+  = markdown(@merge_request.description)
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 6b8d7a6f04..bc375622b3 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -146,7 +146,8 @@ describe Notify do
       end
 
       context 'for issues' do
-        let(:issue) { create(:issue, author: current_user, assignee: assignee, project: project ) }
+        let(:issue) { create(:issue, author: current_user, assignee: assignee, project: project) }
+        let(:issue_with_description) { create(:issue, author: current_user, assignee: assignee, project: project, description: Faker::Lorem.sentence) }
 
         describe 'that are new' do
           subject { Notify.new_issue_email(issue.assignee_id, issue.id) }
@@ -162,6 +163,14 @@ describe Notify do
           end
         end
 
+        describe 'that are new with a description' do
+          subject { Notify.new_issue_email(issue_with_description.assignee_id, issue_with_description.id) }
+
+          it 'contains the description' do
+            should have_body_text /#{issue_with_description.description}/
+          end
+        end
+
         describe 'that have been reassigned' do
           subject { Notify.reassigned_issue_email(recipient.id, issue.id, previous_assignee.id, current_user) }
 
@@ -221,6 +230,7 @@ describe Notify do
 
       context 'for merge requests' do
         let(:merge_request) { create(:merge_request, author: current_user, assignee: assignee, source_project: project, target_project: project) }
+        let(:merge_request_with_description) { create(:merge_request, author: current_user, assignee: assignee, source_project: project, target_project: project, description: Faker::Lorem.sentence) }
 
         describe 'that are new' do
           subject { Notify.new_merge_request_email(merge_request.assignee_id, merge_request.id) }
@@ -244,6 +254,14 @@ describe Notify do
           end
         end
 
+        describe 'that are new with a description' do
+          subject { Notify.new_merge_request_email(merge_request_with_description.assignee_id, merge_request_with_description.id) }
+
+          it 'contains the description' do
+            should have_body_text /#{merge_request_with_description.description}/
+          end
+        end
+
         describe 'that are reassigned' do
           subject { Notify.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) }
 

From eeb011e34a2ccc3ff8a37bd91d8cb7b5cc033228 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Tue, 18 Feb 2014 11:14:04 +0100
Subject: [PATCH 5/9] Remove the author name from issue notes emails

Rationale: the author name is now displayed in the email "From" field;
this information is no longer needed.
---
 app/views/notify/_note_message.html.haml | 4 ----
 spec/mailers/notify_spec.rb              | 4 ----
 2 files changed, 8 deletions(-)

diff --git a/app/views/notify/_note_message.html.haml b/app/views/notify/_note_message.html.haml
index 9e329af2d4..ba2ed5697b 100644
--- a/app/views/notify/_note_message.html.haml
+++ b/app/views/notify/_note_message.html.haml
@@ -1,6 +1,2 @@
-%p
-  %strong #{@note.author_name}
-  wrote:
-
 %cite{style: 'color: #666'}
   = markdown(@note.note)
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index bc375622b3..f990ed659b 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -353,10 +353,6 @@ describe Notify do
           should deliver_to recipient.email
         end
 
-        it 'contains the name of the note\'s author' do
-          should have_body_text /#{note_author.name}/
-        end
-
         it 'contains the message from the note' do
           should have_body_text /#{note.note}/
         end

From ec196fbbf2cb1849631d921015296806d0563da6 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Tue, 18 Feb 2014 12:19:08 +0100
Subject: [PATCH 6/9] Remove the quote-style around message in notification
 emails

Rationale:

* In this notification email, the message is the most important content.
It should not be dimmed or smaller than the rest of the text.

* Now that all UI elements are removed, the message *is* the email. It
should be written as a standard email message, without styling.
---
 app/views/notify/_note_message.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/notify/_note_message.html.haml b/app/views/notify/_note_message.html.haml
index ba2ed5697b..5272dfa0ed 100644
--- a/app/views/notify/_note_message.html.haml
+++ b/app/views/notify/_note_message.html.haml
@@ -1,2 +1,2 @@
-%cite{style: 'color: #666'}
+%div
   = markdown(@note.note)

From 7959455f4d5dc25963176b9f83981b17b415996e Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Tue, 18 Feb 2014 12:14:27 +0100
Subject: [PATCH 7/9] Remove the header banner in notification emails

Rationale:

* The link to the project is already displayed in the message footer.

* Streamlining: the content of the message is better emphasized.
---
 app/views/layouts/notify.html.haml | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/app/views/layouts/notify.html.haml b/app/views/layouts/notify.html.haml
index 9aceb58e8d..c632fdeb3e 100644
--- a/app/views/layouts/notify.html.haml
+++ b/app/views/layouts/notify.html.haml
@@ -5,16 +5,10 @@
         GitLab
 
   %body
-    %h1{style: "background: #EEE; border-bottom: 1px solid #DDD; color: #474D57; font: normal 20px Helvetica, Arial, sans-serif; margin: 0; padding: 5px 10px; line-height: 32px; font-size: 16px;"}
-      GitLab
-      - if @project
-        \|
-        = link_to @project.name_with_namespace, project_url(@project), style: 'color: #29B; text-decoration: none'
     %table{align: "left", border: "0", cellpadding: "0", cellspacing: "0", style: "padding: 10px 0;", width: "100%"}
       %tr
         %td{align: "left", style: "margin: 0; padding: 10px;"}
           = yield
-          %br
       %tr
         %td{align: "left", style: "margin: 0; padding: 10px;"}
           %p{style: "font-size:small;color:#777"}

From 306744bbb0a5e1fc6253ec9512cbae87df2a94c7 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Tue, 18 Feb 2014 14:27:39 +0100
Subject: [PATCH 8/9] Simplify the default layout of notification emails

* Less margin: the content appears as the text of the message.

* Streamlined footer: everything on one line, with a small separator.

* Zimbra (www.zimbra.com) chokes on HTML tables, and doesn't display
the content of the message. Switching to a simpler layout fixes the
issue.
---
 app/views/layouts/notify.html.haml            | 31 ++++++++++++-------
 .../notify/new_merge_request_email.html.haml  |  4 +--
 .../notify/note_merge_request_email.html.haml |  2 +-
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/app/views/layouts/notify.html.haml b/app/views/layouts/notify.html.haml
index c632fdeb3e..09d84a3eb9 100644
--- a/app/views/layouts/notify.html.haml
+++ b/app/views/layouts/notify.html.haml
@@ -3,17 +3,24 @@
     %meta{content: "text/html; charset=utf-8", "http-equiv" => "Content-Type"}
       %title
         GitLab
+  :css
+    p.details {
+      font-style:italic;
+      color:#777
+    }
+    .footer p {
+      font-size:small;
+      color:#777
+    }
 
   %body
-    %table{align: "left", border: "0", cellpadding: "0", cellspacing: "0", style: "padding: 10px 0;", width: "100%"}
-      %tr
-        %td{align: "left", style: "margin: 0; padding: 10px;"}
-          = yield
-      %tr
-        %td{align: "left", style: "margin: 0; padding: 10px;"}
-          %p{style: "font-size:small;color:#777"}
-            - if @project
-              You're receiving this notification because you are a member of the #{link_to @project.name_with_namespace, project_url(@project)} project team.
-              %br
-            - if @target_url
-              #{link_to "View in GitLab", @target_url}
+    %div.content
+      = yield
+    %div.footer{style: "margin-top: 10px;"}
+      %p
+        \—
+        %br
+        - if @project
+          You're receiving this notification because you are a member of the #{link_to @project.name_with_namespace, project_url(@project)} project team.
+        - if @target_url
+          #{link_to "View in GitLab", @target_url}
diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml
index 2c012c4b21..f02d5111b2 100644
--- a/app/views/notify/new_merge_request_email.html.haml
+++ b/app/views/notify/new_merge_request_email.html.haml
@@ -1,6 +1,4 @@
-%p
-  = "New Merge Request ##{@merge_request.iid}"
-%p
+%p.details
   != merge_path_description(@merge_request, '&rarr;')
 
 - if @merge_request.assignee_id.present?
diff --git a/app/views/notify/note_merge_request_email.html.haml b/app/views/notify/note_merge_request_email.html.haml
index 83d07aa0c9..f5287176fb 100644
--- a/app/views/notify/note_merge_request_email.html.haml
+++ b/app/views/notify/note_merge_request_email.html.haml
@@ -1,4 +1,4 @@
-%p
+%p.details
   - if @note.for_diff_line?
     = link_to "New comment on diff", diffs_project_merge_request_url(@merge_request.target_project, @merge_request, anchor: "note_#{@note.id}")
   - else

From ad278c4e7468cc7aac8af27199bc6851853cf779 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Tue, 18 Feb 2014 17:32:12 +0100
Subject: [PATCH 9/9] Make the merge request notification emails more readable

Emphasize the meaningful informations, and remove the less needed ones:

* Use "!" to identify a merge request
* Write the name of the commented file for diff notes
* Don't show the assignee in the "merged merge request" email
---
 .../notify/closed_merge_request_email.html.haml      |  7 +------
 .../notify/merged_merge_request_email.html.haml      |  7 +------
 app/views/notify/note_merge_request_email.html.haml  | 12 ++++++------
 3 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/app/views/notify/closed_merge_request_email.html.haml b/app/views/notify/closed_merge_request_email.html.haml
index 3157f59b2b..809d46f31b 100644
--- a/app/views/notify/closed_merge_request_email.html.haml
+++ b/app/views/notify/closed_merge_request_email.html.haml
@@ -1,7 +1,2 @@
 %p
-  = "Merge Request #{@merge_request.iid} was closed by #{@updated_by.name}"
-%p
-  != merge_path_description(@merge_request, '&rarr;')
-%p
-  Assignee: #{@merge_request.author_name} &rarr; #{@merge_request.assignee_name}
-
+  = "Merge Request !#{@merge_request.iid} was closed by #{@updated_by.name}"
diff --git a/app/views/notify/merged_merge_request_email.html.haml b/app/views/notify/merged_merge_request_email.html.haml
index e3a5239fe0..0c62d439ae 100644
--- a/app/views/notify/merged_merge_request_email.html.haml
+++ b/app/views/notify/merged_merge_request_email.html.haml
@@ -1,7 +1,2 @@
 %p
-  = "Merge Request #{@merge_request.iid} was merged"
-%p
-  != merge_path_description(@merge_request, '&rarr;')
-%p
-  Assignee: #{@merge_request.author_name} &rarr; #{@merge_request.assignee_name}
-
+  = "Merge Request !#{@merge_request.iid} was merged"
diff --git a/app/views/notify/note_merge_request_email.html.haml b/app/views/notify/note_merge_request_email.html.haml
index f5287176fb..65f0e4c406 100644
--- a/app/views/notify/note_merge_request_email.html.haml
+++ b/app/views/notify/note_merge_request_email.html.haml
@@ -1,7 +1,7 @@
-%p.details
-  - if @note.for_diff_line?
-    = link_to "New comment on diff", diffs_project_merge_request_url(@merge_request.target_project, @merge_request, anchor: "note_#{@note.id}")
-  - else
-    = link_to "New comment", project_merge_request_url(@merge_request.target_project, @merge_request, anchor: "note_#{@note.id}")
-  for Merge Request ##{@merge_request.iid}
+- if @note.diff_file_name
+  %p.details
+    New comment on diff for
+    = link_to @note.diff_file_name, @target_url
+    \:
+
 = render 'note_message'