Commit a769204ff489bc750c4931ecdb22ef055eac042a
1 parent
adcfeae1
Exists in
master
and in
4 other branches
Improved commit author notification
Showing
4 changed files
with
50 additions
and
16 deletions
Show diff stats
app/mailers/notify.rb
| @@ -27,8 +27,7 @@ class Notify < ActionMailer::Base | @@ -27,8 +27,7 @@ class Notify < ActionMailer::Base | ||
| 27 | @user = user | 27 | @user = user |
| 28 | @note = note | 28 | @note = note |
| 29 | @project = note.project | 29 | @project = note.project |
| 30 | - @commit = @project.repo.commits(note.noteable_id).first | ||
| 31 | - return unless ( note.notify or ( note.notify_author and @commit.author.email == @user.email ) ) | 30 | + @commit = @note.target |
| 32 | mail(:to => @user.email, :subject => "gitlab | note for commit | #{@note.project.name} ") | 31 | mail(:to => @user.email, :subject => "gitlab | note for commit | #{@note.project.name} ") |
| 33 | end | 32 | end |
| 34 | 33 |
app/models/mailer_observer.rb
| @@ -27,20 +27,25 @@ class MailerObserver < ActiveRecord::Observer | @@ -27,20 +27,25 @@ class MailerObserver < ActiveRecord::Observer | ||
| 27 | end | 27 | end |
| 28 | 28 | ||
| 29 | def new_note(note) | 29 | def new_note(note) |
| 30 | - return unless note.notify or note.notify_author | ||
| 31 | - note.project.users.reject { |u| u.id == current_user.id } .each do |u| | ||
| 32 | - case note.noteable_type | ||
| 33 | - when "Commit" then | ||
| 34 | - Notify.note_commit_email(u, note).deliver | ||
| 35 | - when "Issue" then | ||
| 36 | - Notify.note_issue_email(u, note).deliver | ||
| 37 | - when "MergeRequest" then | ||
| 38 | - Notify.note_merge_request_email(u, note).deliver | ||
| 39 | - when "Snippet" | ||
| 40 | - true | ||
| 41 | - else | ||
| 42 | - Notify.note_wall_email(u, note).deliver | 30 | + # Notify whole team except author of note |
| 31 | + if note.notify | ||
| 32 | + note.project.users.reject { |u| u.id == current_user.id } .each do |u| | ||
| 33 | + case note.noteable_type | ||
| 34 | + when "Commit" then | ||
| 35 | + Notify.note_commit_email(u, note).deliver | ||
| 36 | + when "Issue" then | ||
| 37 | + Notify.note_issue_email(u, note).deliver | ||
| 38 | + when "MergeRequest" then | ||
| 39 | + Notify.note_merge_request_email(u, note).deliver | ||
| 40 | + when "Snippet" | ||
| 41 | + true | ||
| 42 | + else | ||
| 43 | + Notify.note_wall_email(u, note).deliver | ||
| 44 | + end | ||
| 43 | end | 45 | end |
| 46 | + # Notify only author of resource | ||
| 47 | + elsif note.notify_author | ||
| 48 | + Notify.note_commit_email(note.commit_author, note).deliver | ||
| 44 | end | 49 | end |
| 45 | end | 50 | end |
| 46 | 51 |
app/models/note.rb
| @@ -57,6 +57,36 @@ class Note < ActiveRecord::Base | @@ -57,6 +57,36 @@ class Note < ActiveRecord::Base | ||
| 57 | rescue | 57 | rescue |
| 58 | nil | 58 | nil |
| 59 | end | 59 | end |
| 60 | + | ||
| 61 | + # Check if we can notify commit author | ||
| 62 | + # with email about our comment | ||
| 63 | + # | ||
| 64 | + # If commit author email exist in project | ||
| 65 | + # and commit author is not passed user we can | ||
| 66 | + # send email to him | ||
| 67 | + # | ||
| 68 | + # params: | ||
| 69 | + # user - current user | ||
| 70 | + # | ||
| 71 | + # return: | ||
| 72 | + # Boolean | ||
| 73 | + # | ||
| 74 | + def notify_only_author?(user) | ||
| 75 | + commit? && commit_author && | ||
| 76 | + commit_author.email != user.email | ||
| 77 | + end | ||
| 78 | + | ||
| 79 | + def commit? | ||
| 80 | + noteable_type == "Commit" | ||
| 81 | + end | ||
| 82 | + | ||
| 83 | + def commit_author | ||
| 84 | + @commit_author ||= | ||
| 85 | + project.users.find_by_email(target.author_email) || | ||
| 86 | + project.users.find_by_name(target.author_name) | ||
| 87 | + rescue | ||
| 88 | + nil | ||
| 89 | + end | ||
| 60 | end | 90 | end |
| 61 | # == Schema Information | 91 | # == Schema Information |
| 62 | # | 92 | # |
app/views/notes/_form.html.haml
| @@ -17,7 +17,7 @@ | @@ -17,7 +17,7 @@ | ||
| 17 | = check_box_tag :notify, 1, @note.noteable_type != "Commit" | 17 | = check_box_tag :notify, 1, @note.noteable_type != "Commit" |
| 18 | %span Project team | 18 | %span Project team |
| 19 | 19 | ||
| 20 | - -if @note.noteable_type == "Commit" | 20 | + - if @note.notify_only_author?(current_user) |
| 21 | = label_tag :notify_author do | 21 | = label_tag :notify_author do |
| 22 | = check_box_tag :notify_author, 1 , @note.noteable_type == "Commit" | 22 | = check_box_tag :notify_author, 1 , @note.noteable_type == "Commit" |
| 23 | %span Commit author | 23 | %span Commit author |