Commit 6df16564efd712ca201428504dde5b6ac020a87d
Exists in
master
and in
4 other branches
Merge pull request #5076 from jzi/notify_commit_author
Re-enable notifying commit authors of new commit notes
Showing
2 changed files
with
19 additions
and
18 deletions
Show diff stats
app/services/notification_service.rb
| @@ -107,12 +107,6 @@ class NotificationService | @@ -107,12 +107,6 @@ class NotificationService | ||
| 107 | 107 | ||
| 108 | opts = { noteable_type: note.noteable_type, project_id: note.project_id } | 108 | opts = { noteable_type: note.noteable_type, project_id: note.project_id } |
| 109 | 109 | ||
| 110 | - if note.commit_id.present? | ||
| 111 | - opts.merge!(commit_id: note.commit_id) | ||
| 112 | - else | ||
| 113 | - opts.merge!(noteable_id: note.noteable_id) | ||
| 114 | - end | ||
| 115 | - | ||
| 116 | target = note.noteable | 110 | target = note.noteable |
| 117 | if target.respond_to?(:participants) | 111 | if target.respond_to?(:participants) |
| 118 | recipients = target.participants | 112 | recipients = target.participants |
| @@ -120,6 +114,13 @@ class NotificationService | @@ -120,6 +114,13 @@ class NotificationService | ||
| 120 | recipients = note.mentioned_users | 114 | recipients = note.mentioned_users |
| 121 | end | 115 | end |
| 122 | 116 | ||
| 117 | + if note.commit_id.present? | ||
| 118 | + opts.merge!(commit_id: note.commit_id) | ||
| 119 | + recipients << note.commit_author | ||
| 120 | + else | ||
| 121 | + opts.merge!(noteable_id: note.noteable_id) | ||
| 122 | + end | ||
| 123 | + | ||
| 123 | # Get users who left comment in thread | 124 | # Get users who left comment in thread |
| 124 | recipients = recipients.concat(User.where(id: Note.where(opts).pluck(:author_id))) | 125 | recipients = recipients.concat(User.where(id: Note.where(opts).pluck(:author_id))) |
| 125 | 126 |
spec/services/notification_service_spec.rb
| @@ -52,10 +52,12 @@ describe NotificationService do | @@ -52,10 +52,12 @@ describe NotificationService do | ||
| 52 | 52 | ||
| 53 | before do | 53 | before do |
| 54 | build_team(note.project) | 54 | build_team(note.project) |
| 55 | + note.stub(:commit_author => @u_committer) | ||
| 55 | end | 56 | end |
| 56 | 57 | ||
| 57 | describe :new_note do | 58 | describe :new_note do |
| 58 | it do | 59 | it do |
| 60 | + should_email(@u_committer.id, note) | ||
| 59 | should_email(@u_watcher.id, note) | 61 | should_email(@u_watcher.id, note) |
| 60 | should_not_email(@u_mentioned.id, note) | 62 | should_not_email(@u_mentioned.id, note) |
| 61 | should_not_email(note.author_id, note) | 63 | should_not_email(note.author_id, note) |
| @@ -65,18 +67,14 @@ describe NotificationService do | @@ -65,18 +67,14 @@ describe NotificationService do | ||
| 65 | end | 67 | end |
| 66 | 68 | ||
| 67 | it do | 69 | it do |
| 68 | - new_note = create(:note_on_commit, | ||
| 69 | - author: @u_participating, | ||
| 70 | - project_id: note.project_id, | ||
| 71 | - commit_id: note.commit_id, | ||
| 72 | - note: '@mention referenced') | ||
| 73 | - | ||
| 74 | - should_email(@u_watcher.id, new_note) | ||
| 75 | - should_email(@u_mentioned.id, new_note) | ||
| 76 | - should_not_email(new_note.author_id, new_note) | ||
| 77 | - should_not_email(@u_participating.id, new_note) | ||
| 78 | - should_not_email(@u_disabled.id, new_note) | ||
| 79 | - notification.new_note(new_note) | 70 | + note.update_attribute(:note, '@mention referenced') |
| 71 | + should_email(@u_committer.id, note) | ||
| 72 | + should_email(@u_watcher.id, note) | ||
| 73 | + should_email(@u_mentioned.id, note) | ||
| 74 | + should_not_email(note.author_id, note) | ||
| 75 | + should_not_email(@u_participating.id, note) | ||
| 76 | + should_not_email(@u_disabled.id, note) | ||
| 77 | + notification.new_note(note) | ||
| 80 | end | 78 | end |
| 81 | 79 | ||
| 82 | def should_email(user_id, n) | 80 | def should_email(user_id, n) |
| @@ -240,10 +238,12 @@ describe NotificationService do | @@ -240,10 +238,12 @@ describe NotificationService do | ||
| 240 | @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING) | 238 | @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING) |
| 241 | @u_disabled = create(:user, notification_level: Notification::N_DISABLED) | 239 | @u_disabled = create(:user, notification_level: Notification::N_DISABLED) |
| 242 | @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_PARTICIPATING) | 240 | @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_PARTICIPATING) |
| 241 | + @u_committer = create(:user, username: 'committer') | ||
| 243 | 242 | ||
| 244 | project.team << [@u_watcher, :master] | 243 | project.team << [@u_watcher, :master] |
| 245 | project.team << [@u_participating, :master] | 244 | project.team << [@u_participating, :master] |
| 246 | project.team << [@u_disabled, :master] | 245 | project.team << [@u_disabled, :master] |
| 247 | project.team << [@u_mentioned, :master] | 246 | project.team << [@u_mentioned, :master] |
| 247 | + project.team << [@u_committer, :master] | ||
| 248 | end | 248 | end |
| 249 | end | 249 | end |