Commit b6d0dd871c7c8caa4809e1e8b60f6e4bb66f38c2
Exists in
master
and in
4 other branches
Merge pull request #4618 from jacargentina/notification-on-commits
Fix notifications to handle participants and mentions on commits too
Showing
2 changed files
with
28 additions
and
25 deletions
Show diff stats
app/services/notification_service.rb
@@ -106,15 +106,15 @@ class NotificationService | @@ -106,15 +106,15 @@ class NotificationService | ||
106 | 106 | ||
107 | if note.commit_id.present? | 107 | if note.commit_id.present? |
108 | opts.merge!(commit_id: note.commit_id) | 108 | opts.merge!(commit_id: note.commit_id) |
109 | - recipients = [note.commit_author] | ||
110 | else | 109 | else |
111 | opts.merge!(noteable_id: note.noteable_id) | 110 | opts.merge!(noteable_id: note.noteable_id) |
112 | - target = note.noteable | ||
113 | - if target.respond_to?(:participants) | ||
114 | - recipients = target.participants | ||
115 | - else | ||
116 | - recipients = [] | ||
117 | - end | 111 | + end |
112 | + | ||
113 | + target = note.noteable | ||
114 | + if target.respond_to?(:participants) | ||
115 | + recipients = target.participants | ||
116 | + else | ||
117 | + recipients = note.mentioned_users | ||
118 | end | 118 | end |
119 | 119 | ||
120 | # Get users who left comment in thread | 120 | # Get users who left comment in thread |
spec/services/notification_service_spec.rb
@@ -48,7 +48,7 @@ describe NotificationService do | @@ -48,7 +48,7 @@ describe NotificationService do | ||
48 | end | 48 | end |
49 | 49 | ||
50 | context 'commit note' do | 50 | context 'commit note' do |
51 | - let(:note) { create :note_on_commit } | 51 | + let(:note) { create(:note_on_commit) } |
52 | 52 | ||
53 | before do | 53 | before do |
54 | build_team(note.project) | 54 | build_team(note.project) |
@@ -56,32 +56,35 @@ describe NotificationService do | @@ -56,32 +56,35 @@ describe NotificationService do | ||
56 | 56 | ||
57 | describe :new_note do | 57 | describe :new_note do |
58 | it do | 58 | it do |
59 | - should_email(@u_watcher.id) | ||
60 | - should_not_email(note.author_id) | ||
61 | - should_not_email(@u_participating.id) | ||
62 | - should_not_email(@u_disabled.id) | 59 | + should_email(@u_watcher.id, note) |
60 | + should_not_email(@u_mentioned.id, note) | ||
61 | + should_not_email(note.author_id, note) | ||
62 | + should_not_email(@u_participating.id, note) | ||
63 | + should_not_email(@u_disabled.id, note) | ||
63 | notification.new_note(note) | 64 | notification.new_note(note) |
64 | end | 65 | end |
65 | 66 | ||
66 | it do | 67 | it do |
67 | - create(:note_on_commit, | 68 | + new_note = create(:note_on_commit, |
68 | author: @u_participating, | 69 | author: @u_participating, |
69 | project_id: note.project_id, | 70 | project_id: note.project_id, |
70 | - commit_id: note.commit_id) | ||
71 | - | ||
72 | - should_email(@u_watcher.id) | ||
73 | - should_email(@u_participating.id) | ||
74 | - should_not_email(note.author_id) | ||
75 | - should_not_email(@u_disabled.id) | ||
76 | - notification.new_note(note) | 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) | ||
77 | end | 80 | end |
78 | 81 | ||
79 | - def should_email(user_id) | ||
80 | - Notify.should_receive(:note_commit_email).with(user_id, note.id) | 82 | + def should_email(user_id, n) |
83 | + Notify.should_receive(:note_commit_email).with(user_id, n.id) | ||
81 | end | 84 | end |
82 | 85 | ||
83 | - def should_not_email(user_id) | ||
84 | - Notify.should_not_receive(:note_commit_email).with(user_id, note.id) | 86 | + def should_not_email(user_id, n) |
87 | + Notify.should_not_receive(:note_commit_email).with(user_id, n.id) | ||
85 | end | 88 | end |
86 | end | 89 | end |
87 | end | 90 | end |
@@ -236,7 +239,7 @@ describe NotificationService do | @@ -236,7 +239,7 @@ describe NotificationService do | ||
236 | @u_watcher = create(:user, notification_level: Notification::N_WATCH) | 239 | @u_watcher = create(:user, notification_level: Notification::N_WATCH) |
237 | @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING) | 240 | @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING) |
238 | @u_disabled = create(:user, notification_level: Notification::N_DISABLED) | 241 | @u_disabled = create(:user, notification_level: Notification::N_DISABLED) |
239 | - @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_WATCH) | 242 | + @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_PARTICIPATING) |
240 | 243 | ||
241 | project.team << [@u_watcher, :master] | 244 | project.team << [@u_watcher, :master] |
242 | project.team << [@u_participating, :master] | 245 | project.team << [@u_participating, :master] |