diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2014-03-11 14:39:29 +0400 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2014-03-11 14:39:29 +0400 |
commit | c4f9dff480ea030d6878b0197a71670f3a141e0e (patch) | |
tree | d351e23dfe82247acc12dcc8319a3e0541f0a821 /spec | |
parent | dcda7516d6657e1372137e5549e3af4400e0ad5f (diff) | |
parent | ad278c4e7468cc7aac8af27199bc6851853cf779 (diff) |
Merge branch 'simplify-emails-content' into 'master'
Streamline the content of notification emails
In notification emails, the actual content of the email is often buried under several blocks of chrome — and may even be truncated or completely missing. Ideally, the notification emails would be like *real emails*: a short message of meaningful text, sent from the author of the change that triggered the notification.
This MR includes the following changes to notification emails:
* Remove much of the chrome (e.g. the "GitLab" header)
* Emphasize the content (no more small, grayed-out content)
* Add missing informations to the emails (issue description in "new issue" email, file name in "diff comment" email)
* Add a consistent "View in GitLab" link in the footer
* The assignee is displayed only if someone is assigned
* Fix a rendering bug when viewing emails with [Zimbra](http://www.zimbra.com/)
We use these patches at [Capitaine Train](http://www.capitainetrain.com), and it has been a surprisingly big productivity boost for us.
![Before and after](http://f.cl.ly/items/3n0P2c2v1P0y011c0D3e/Before%20and%20After.png)
Diffstat (limited to 'spec')
-rw-r--r-- | spec/mailers/notify_spec.rb | 30 |
1 files changed, 25 insertions, 5 deletions
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 6ba4d97ad4a..f990ed659b8 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) } @@ -335,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 @@ -468,6 +482,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 +508,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 |