diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb index d89e7734..355f7848 100644 --- a/app/observers/merge_request_observer.rb +++ b/app/observers/merge_request_observer.rb @@ -2,9 +2,7 @@ class MergeRequestObserver < ActiveRecord::Observer cattr_accessor :current_user def after_create(merge_request) - if merge_request.assignee && merge_request.assignee != current_user - Notify.delay.new_merge_request_email(merge_request.id) - end + notification.new_merge_request(merge_request, current_user) end def after_close(merge_request, transition) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 804b33e0..7e7a36f3 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -47,7 +47,7 @@ class NotificationService end end - # When we reassign an issue we should send next emails: + # When create an issue we should send next emails: # # * issue assignee if his notification level is not Disabled # @@ -56,4 +56,14 @@ class NotificationService Notify.delay.new_issue_email(issue.id) end end + + # When create a merge request we should send next emails: + # + # * mr assignee if his notification level is not Disabled + # + def new_merge_request(merge_request, current_user) + if merge_request.assignee && merge_request.assignee != current_user + Notify.delay.new_merge_request_email(merge_request.id) + end + end end diff --git a/spec/observers/merge_request_observer_spec.rb b/spec/observers/merge_request_observer_spec.rb index 9d702107..991f2d40 100644 --- a/spec/observers/merge_request_observer_spec.rb +++ b/spec/observers/merge_request_observer_spec.rb @@ -10,7 +10,8 @@ describe MergeRequestObserver do let(:closed_assigned_mr) { create(:closed_merge_request, assignee: assignee, author: author) } let(:closed_unassigned_mr) { create(:closed_merge_request, author: author) } - before(:each) { subject.stub(:current_user).and_return(some_user) } + before { subject.stub(:current_user).and_return(some_user) } + before { subject.stub(notification: mock('NotificationService').as_null_object) } subject { MergeRequestObserver.instance } @@ -18,21 +19,11 @@ describe MergeRequestObserver do it 'is called when a merge request is created' do subject.should_receive(:after_create) - - MergeRequest.observers.enable :merge_request_observer do - create(:merge_request, project: create(:project)) - end + create(:merge_request, project: create(:project)) end - it 'sends an email to the assignee' do - Notify.should_receive(:new_merge_request_email).with(mr_mock.id) - subject.after_create(mr_mock) - end - - it 'does not send an email to the assignee if assignee created the merge request' do - subject.stub(:current_user).and_return(assignee) - Notify.should_not_receive(:new_merge_request_email) - + it 'trigger notification service' do + subject.should_receive(:notification) subject.after_create(mr_mock) end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 0edfb216..e9e4770d 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -44,4 +44,20 @@ describe NotificationService do end end end + + describe 'Merge Requests' do + let(:merge_request) { create :merge_request, assignee: create(:user) } + + describe :new_merge_request do + it 'should send email to merge_request assignee' do + Notify.should_receive(:new_merge_request_email).with(merge_request.id) + notification.new_merge_request(merge_request, merge_request.author) + end + + it 'should not send email to merge_request assignee if he is current_user' do + Notify.should_not_receive(:new_merge_request_email).with(merge_request.id) + notification.new_merge_request(merge_request, merge_request.assignee) + end + end + end end