From cb877b7e6f6b036a2811aec7313dab94473774de Mon Sep 17 00:00:00 2001 From: Javier Castro Date: Mon, 22 Jul 2013 15:45:56 -0300 Subject: [PATCH 1/2] Fix notifications to handle participants and mentions on commits too --- app/services/notification_service.rb | 14 +++++++------- spec/services/notification_service_spec.rb | 7 +++++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index f3dc552a8e..819b76c279 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -106,15 +106,15 @@ class NotificationService if note.commit_id.present? opts.merge!(commit_id: note.commit_id) - recipients = [note.commit_author] else opts.merge!(noteable_id: note.noteable_id) - target = note.noteable - if target.respond_to?(:participants) - recipients = target.participants - else - recipients = [] - end + end + + target = note.noteable + if target.respond_to?(:participants) + recipients = target.participants + else + recipients = note.mentioned_users end # Get users who left comment in thread diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 7650148230..8ebad41b46 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -48,7 +48,7 @@ describe NotificationService do end context 'commit note' do - let(:note) { create :note_on_commit } + let(:note) { create(:note_on_commit, note: '@mention referenced') } before do build_team(note.project) @@ -57,6 +57,7 @@ describe NotificationService do describe :new_note do it do should_email(@u_watcher.id) + should_email(@u_mentioned.id) should_not_email(note.author_id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) @@ -67,10 +68,12 @@ describe NotificationService do create(:note_on_commit, author: @u_participating, project_id: note.project_id, - commit_id: note.commit_id) + commit_id: note.commit_id, + note: '@mention referenced') should_email(@u_watcher.id) should_email(@u_participating.id) + should_email(@u_mentioned.id) should_not_email(note.author_id) should_not_email(@u_disabled.id) notification.new_note(note) From 669ada924b3d058f5ad8bfc6822b1a9e64b3549b Mon Sep 17 00:00:00 2001 From: Javier Castro Date: Mon, 29 Jul 2013 17:34:53 -0300 Subject: [PATCH 2/2] Fix test case for notification_service --- spec/services/notification_service_spec.rb | 36 +++++++++++----------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 8ebad41b46..0254725443 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -48,7 +48,7 @@ describe NotificationService do end context 'commit note' do - let(:note) { create(:note_on_commit, note: '@mention referenced') } + let(:note) { create(:note_on_commit) } before do build_team(note.project) @@ -56,35 +56,35 @@ describe NotificationService do describe :new_note do it do - should_email(@u_watcher.id) - should_email(@u_mentioned.id) - should_not_email(note.author_id) - should_not_email(@u_participating.id) - should_not_email(@u_disabled.id) + should_email(@u_watcher.id, note) + should_not_email(@u_mentioned.id, note) + should_not_email(note.author_id, note) + should_not_email(@u_participating.id, note) + should_not_email(@u_disabled.id, note) notification.new_note(note) end it do - create(:note_on_commit, + new_note = create(:note_on_commit, author: @u_participating, project_id: note.project_id, commit_id: note.commit_id, note: '@mention referenced') - should_email(@u_watcher.id) - should_email(@u_participating.id) - should_email(@u_mentioned.id) - should_not_email(note.author_id) - should_not_email(@u_disabled.id) - notification.new_note(note) + should_email(@u_watcher.id, new_note) + should_email(@u_mentioned.id, new_note) + should_not_email(new_note.author_id, new_note) + should_not_email(@u_participating.id, new_note) + should_not_email(@u_disabled.id, new_note) + notification.new_note(new_note) end - def should_email(user_id) - Notify.should_receive(:note_commit_email).with(user_id, note.id) + def should_email(user_id, n) + Notify.should_receive(:note_commit_email).with(user_id, n.id) end - def should_not_email(user_id) - Notify.should_not_receive(:note_commit_email).with(user_id, note.id) + def should_not_email(user_id, n) + Notify.should_not_receive(:note_commit_email).with(user_id, n.id) end end end @@ -239,7 +239,7 @@ describe NotificationService do @u_watcher = create(:user, notification_level: Notification::N_WATCH) @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING) @u_disabled = create(:user, notification_level: Notification::N_DISABLED) - @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_WATCH) + @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_PARTICIPATING) project.team << [@u_watcher, :master] project.team << [@u_participating, :master]