Commit 7ba0b502d3e6fdc5efc5619d803a1b9fcf9db791
1 parent
03920b96
Exists in
spb-stable
and in
3 other branches
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.
Showing
8 changed files
with
31 additions
and
1 deletions
Show diff stats
app/mailers/emails/groups.rb
@@ -3,7 +3,7 @@ module Emails | @@ -3,7 +3,7 @@ module Emails | ||
3 | def group_access_granted_email(user_group_id) | 3 | def group_access_granted_email(user_group_id) |
4 | @membership = UsersGroup.find(user_group_id) | 4 | @membership = UsersGroup.find(user_group_id) |
5 | @group = @membership.group | 5 | @group = @membership.group |
6 | - | 6 | + @target_url = group_url(@group) |
7 | mail(to: @membership.user.email, | 7 | mail(to: @membership.user.email, |
8 | subject: subject("Access to group was granted")) | 8 | subject: subject("Access to group was granted")) |
9 | end | 9 | end |
app/mailers/emails/issues.rb
@@ -3,6 +3,7 @@ module Emails | @@ -3,6 +3,7 @@ module Emails | ||
3 | def new_issue_email(recipient_id, issue_id) | 3 | def new_issue_email(recipient_id, issue_id) |
4 | @issue = Issue.find(issue_id) | 4 | @issue = Issue.find(issue_id) |
5 | @project = @issue.project | 5 | @project = @issue.project |
6 | + @target_url = project_issue_url(@project, @issue) | ||
6 | mail(from: sender(@issue.author_id), | 7 | mail(from: sender(@issue.author_id), |
7 | to: recipient(recipient_id), | 8 | to: recipient(recipient_id), |
8 | subject: subject("#{@issue.title} (##{@issue.iid})")) | 9 | subject: subject("#{@issue.title} (##{@issue.iid})")) |
@@ -12,6 +13,7 @@ module Emails | @@ -12,6 +13,7 @@ module Emails | ||
12 | @issue = Issue.find(issue_id) | 13 | @issue = Issue.find(issue_id) |
13 | @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id | 14 | @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id |
14 | @project = @issue.project | 15 | @project = @issue.project |
16 | + @target_url = project_issue_url(@project, @issue) | ||
15 | mail(from: sender(updated_by_user_id), | 17 | mail(from: sender(updated_by_user_id), |
16 | to: recipient(recipient_id), | 18 | to: recipient(recipient_id), |
17 | subject: subject("#{@issue.title} (##{@issue.iid})")) | 19 | subject: subject("#{@issue.title} (##{@issue.iid})")) |
@@ -21,6 +23,7 @@ module Emails | @@ -21,6 +23,7 @@ module Emails | ||
21 | @issue = Issue.find issue_id | 23 | @issue = Issue.find issue_id |
22 | @project = @issue.project | 24 | @project = @issue.project |
23 | @updated_by = User.find updated_by_user_id | 25 | @updated_by = User.find updated_by_user_id |
26 | + @target_url = project_issue_url(@project, @issue) | ||
24 | mail(from: sender(updated_by_user_id), | 27 | mail(from: sender(updated_by_user_id), |
25 | to: recipient(recipient_id), | 28 | to: recipient(recipient_id), |
26 | subject: subject("#{@issue.title} (##{@issue.iid})")) | 29 | subject: subject("#{@issue.title} (##{@issue.iid})")) |
@@ -31,6 +34,7 @@ module Emails | @@ -31,6 +34,7 @@ module Emails | ||
31 | @issue_status = status | 34 | @issue_status = status |
32 | @project = @issue.project | 35 | @project = @issue.project |
33 | @updated_by = User.find updated_by_user_id | 36 | @updated_by = User.find updated_by_user_id |
37 | + @target_url = project_issue_url(@project, @issue) | ||
34 | mail(from: sender(updated_by_user_id), | 38 | mail(from: sender(updated_by_user_id), |
35 | to: recipient(recipient_id), | 39 | to: recipient(recipient_id), |
36 | subject: subject("#{@issue.title} (##{@issue.iid})")) | 40 | subject: subject("#{@issue.title} (##{@issue.iid})")) |
app/mailers/emails/merge_requests.rb
@@ -3,6 +3,7 @@ module Emails | @@ -3,6 +3,7 @@ module Emails | ||
3 | def new_merge_request_email(recipient_id, merge_request_id) | 3 | def new_merge_request_email(recipient_id, merge_request_id) |
4 | @merge_request = MergeRequest.find(merge_request_id) | 4 | @merge_request = MergeRequest.find(merge_request_id) |
5 | @project = @merge_request.project | 5 | @project = @merge_request.project |
6 | + @target_url = project_merge_request_url(@project, @merge_request) | ||
6 | mail(from: sender(@merge_request.author_id), | 7 | mail(from: sender(@merge_request.author_id), |
7 | to: recipient(recipient_id), | 8 | to: recipient(recipient_id), |
8 | subject: subject("#{@merge_request.title} (!#{@merge_request.iid})")) | 9 | subject: subject("#{@merge_request.title} (!#{@merge_request.iid})")) |
@@ -12,6 +13,7 @@ module Emails | @@ -12,6 +13,7 @@ module Emails | ||
12 | @merge_request = MergeRequest.find(merge_request_id) | 13 | @merge_request = MergeRequest.find(merge_request_id) |
13 | @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id | 14 | @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id |
14 | @project = @merge_request.project | 15 | @project = @merge_request.project |
16 | + @target_url = project_merge_request_url(@project, @merge_request) | ||
15 | mail(from: sender(updated_by_user_id), | 17 | mail(from: sender(updated_by_user_id), |
16 | to: recipient(recipient_id), | 18 | to: recipient(recipient_id), |
17 | subject: subject("#{@merge_request.title} (!#{@merge_request.iid})")) | 19 | subject: subject("#{@merge_request.title} (!#{@merge_request.iid})")) |
@@ -21,6 +23,7 @@ module Emails | @@ -21,6 +23,7 @@ module Emails | ||
21 | @merge_request = MergeRequest.find(merge_request_id) | 23 | @merge_request = MergeRequest.find(merge_request_id) |
22 | @updated_by = User.find updated_by_user_id | 24 | @updated_by = User.find updated_by_user_id |
23 | @project = @merge_request.project | 25 | @project = @merge_request.project |
26 | + @target_url = project_merge_request_url(@project, @merge_request) | ||
24 | mail(from: sender(updated_by_user_id), | 27 | mail(from: sender(updated_by_user_id), |
25 | to: recipient(recipient_id), | 28 | to: recipient(recipient_id), |
26 | subject: subject("#{@merge_request.title} (!#{@merge_request.iid})")) | 29 | subject: subject("#{@merge_request.title} (!#{@merge_request.iid})")) |
@@ -29,6 +32,7 @@ module Emails | @@ -29,6 +32,7 @@ module Emails | ||
29 | def merged_merge_request_email(recipient_id, merge_request_id) | 32 | def merged_merge_request_email(recipient_id, merge_request_id) |
30 | @merge_request = MergeRequest.find(merge_request_id) | 33 | @merge_request = MergeRequest.find(merge_request_id) |
31 | @project = @merge_request.project | 34 | @project = @merge_request.project |
35 | + @target_url = project_merge_request_url(@project, @merge_request) | ||
32 | mail(from: sender(@merge_request.author_id_of_changes), | 36 | mail(from: sender(@merge_request.author_id_of_changes), |
33 | to: recipient(recipient_id), | 37 | to: recipient(recipient_id), |
34 | subject: subject("#{@merge_request.title} (!#{@merge_request.iid})")) | 38 | subject: subject("#{@merge_request.title} (!#{@merge_request.iid})")) |
app/mailers/emails/notes.rb
@@ -4,6 +4,7 @@ module Emails | @@ -4,6 +4,7 @@ module Emails | ||
4 | @note = Note.find(note_id) | 4 | @note = Note.find(note_id) |
5 | @commit = @note.noteable | 5 | @commit = @note.noteable |
6 | @project = @note.project | 6 | @project = @note.project |
7 | + @target_url = project_commit_url(@project, @commit, anchor: "note_#{@note.id}") | ||
7 | mail(from: sender(@note.author_id), | 8 | mail(from: sender(@note.author_id), |
8 | to: recipient(recipient_id), | 9 | to: recipient(recipient_id), |
9 | subject: subject("#{@commit.title} (#{@commit.short_id})")) | 10 | subject: subject("#{@commit.title} (#{@commit.short_id})")) |
@@ -13,6 +14,7 @@ module Emails | @@ -13,6 +14,7 @@ module Emails | ||
13 | @note = Note.find(note_id) | 14 | @note = Note.find(note_id) |
14 | @issue = @note.noteable | 15 | @issue = @note.noteable |
15 | @project = @note.project | 16 | @project = @note.project |
17 | + @target_url = project_issue_url(@project, @issue) | ||
16 | mail(from: sender(@note.author_id), | 18 | mail(from: sender(@note.author_id), |
17 | to: recipient(recipient_id), | 19 | to: recipient(recipient_id), |
18 | subject: subject("#{@issue.title} (##{@issue.iid})")) | 20 | subject: subject("#{@issue.title} (##{@issue.iid})")) |
@@ -22,6 +24,7 @@ module Emails | @@ -22,6 +24,7 @@ module Emails | ||
22 | @note = Note.find(note_id) | 24 | @note = Note.find(note_id) |
23 | @merge_request = @note.noteable | 25 | @merge_request = @note.noteable |
24 | @project = @note.project | 26 | @project = @note.project |
27 | + @target_url = project_merge_request_url(@project, @merge_request, anchor: "note_#{@note.id}") | ||
25 | mail(from: sender(@note.author_id), | 28 | mail(from: sender(@note.author_id), |
26 | to: recipient(recipient_id), | 29 | to: recipient(recipient_id), |
27 | subject: subject("#{@merge_request.title} (!#{@merge_request.iid})")) | 30 | subject: subject("#{@merge_request.title} (!#{@merge_request.iid})")) |
@@ -30,6 +33,7 @@ module Emails | @@ -30,6 +33,7 @@ module Emails | ||
30 | def note_wall_email(recipient_id, note_id) | 33 | def note_wall_email(recipient_id, note_id) |
31 | @note = Note.find(note_id) | 34 | @note = Note.find(note_id) |
32 | @project = @note.project | 35 | @project = @note.project |
36 | + @target_url = project_wall_url(@note.project, anchor: "note_#{@note.id}") | ||
33 | mail(from: sender(@note.author_id), | 37 | mail(from: sender(@note.author_id), |
34 | to: recipient(recipient_id), | 38 | to: recipient(recipient_id), |
35 | subject: subject("Note on wall")) | 39 | subject: subject("Note on wall")) |
app/mailers/emails/profile.rb
@@ -3,6 +3,7 @@ module Emails | @@ -3,6 +3,7 @@ module Emails | ||
3 | def new_user_email(user_id, password) | 3 | def new_user_email(user_id, password) |
4 | @user = User.find(user_id) | 4 | @user = User.find(user_id) |
5 | @password = password | 5 | @password = password |
6 | + @target_url = user_url(@user) | ||
6 | mail(to: @user.email, subject: subject("Account was created for you")) | 7 | mail(to: @user.email, subject: subject("Account was created for you")) |
7 | end | 8 | end |
8 | 9 | ||
@@ -15,6 +16,7 @@ module Emails | @@ -15,6 +16,7 @@ module Emails | ||
15 | def new_ssh_key_email(key_id) | 16 | def new_ssh_key_email(key_id) |
16 | @key = Key.find(key_id) | 17 | @key = Key.find(key_id) |
17 | @user = @key.user | 18 | @user = @key.user |
19 | + @target_url = user_url(@user) | ||
18 | mail(to: @user.email, subject: subject("SSH key was added to your account")) | 20 | mail(to: @user.email, subject: subject("SSH key was added to your account")) |
19 | end | 21 | end |
20 | end | 22 | end |
app/mailers/emails/projects.rb
@@ -3,6 +3,7 @@ module Emails | @@ -3,6 +3,7 @@ module Emails | ||
3 | def project_access_granted_email(user_project_id) | 3 | def project_access_granted_email(user_project_id) |
4 | @users_project = UsersProject.find user_project_id | 4 | @users_project = UsersProject.find user_project_id |
5 | @project = @users_project.project | 5 | @project = @users_project.project |
6 | + @target_url = project_url(@project) | ||
6 | mail(to: @users_project.user.email, | 7 | mail(to: @users_project.user.email, |
7 | subject: subject("Access to project was granted")) | 8 | subject: subject("Access to project was granted")) |
8 | end | 9 | end |
@@ -10,6 +11,7 @@ module Emails | @@ -10,6 +11,7 @@ module Emails | ||
10 | def project_was_moved_email(project_id, user_id) | 11 | def project_was_moved_email(project_id, user_id) |
11 | @user = User.find user_id | 12 | @user = User.find user_id |
12 | @project = Project.find project_id | 13 | @project = Project.find project_id |
14 | + @target_url = project_url(@project) | ||
13 | mail(to: @user.email, | 15 | mail(to: @user.email, |
14 | subject: subject("Project was moved")) | 16 | subject: subject("Project was moved")) |
15 | end | 17 | end |
@@ -21,6 +23,11 @@ module Emails | @@ -21,6 +23,11 @@ module Emails | ||
21 | @commits = Commit.decorate(compare.commits) | 23 | @commits = Commit.decorate(compare.commits) |
22 | @diffs = compare.diffs | 24 | @diffs = compare.diffs |
23 | @branch = branch | 25 | @branch = branch |
26 | + if @commits.length > 1 | ||
27 | + @target_url = project_compare_url(@project, from: @commits.first, to: @commits.last) | ||
28 | + else | ||
29 | + @target_url = project_commit_url(@project, @compare.commit) | ||
30 | + end | ||
24 | 31 | ||
25 | mail(from: sender(author_id), | 32 | mail(from: sender(author_id), |
26 | to: recipient, | 33 | to: recipient, |
app/views/layouts/notify.html.haml
@@ -20,3 +20,6 @@ | @@ -20,3 +20,6 @@ | ||
20 | %p{style: "font-size:small;color:#777"} | 20 | %p{style: "font-size:small;color:#777"} |
21 | - if @project | 21 | - if @project |
22 | You're receiving this notification because you are a member of the #{@project.name_with_namespace} project team. | 22 | You're receiving this notification because you are a member of the #{@project.name_with_namespace} project team. |
23 | + %br | ||
24 | + - if @target_url | ||
25 | + #{link_to "View in GitLab", @target_url} |
spec/mailers/notify_spec.rb
@@ -468,6 +468,8 @@ describe Notify do | @@ -468,6 +468,8 @@ describe Notify do | ||
468 | let(:example_site_path) { root_path } | 468 | let(:example_site_path) { root_path } |
469 | let(:user) { create(:user) } | 469 | let(:user) { create(:user) } |
470 | let(:compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, 'cd5c4bac', 'b1e6a9db') } | 470 | let(:compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, 'cd5c4bac', 'b1e6a9db') } |
471 | + let(:commits) { Commit.decorate(compare.commits) } | ||
472 | + let(:diff_path) { project_compare_path(project, from: commits.first, to: commits.last) } | ||
471 | 473 | ||
472 | subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'master', compare) } | 474 | subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'master', compare) } |
473 | 475 | ||
@@ -492,5 +494,9 @@ describe Notify do | @@ -492,5 +494,9 @@ describe Notify do | ||
492 | it 'includes diffs' do | 494 | it 'includes diffs' do |
493 | should have_body_text /Checkout wiki pages for installation information/ | 495 | should have_body_text /Checkout wiki pages for installation information/ |
494 | end | 496 | end |
497 | + | ||
498 | + it 'contains a link to the diff' do | ||
499 | + should have_body_text /#{diff_path}/ | ||
500 | + end | ||
495 | end | 501 | end |
496 | end | 502 | end |