From 3728c4904e61e47d23b6454754451bd716f4f422 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 26 Mar 2013 19:00:54 +0200 Subject: [PATCH] refactor observers test since email logic moved to service --- app/observers/activity_observer.rb | 2 +- app/observers/base_observer.rb | 2 - app/services/notification_service.rb | 8 ++ spec/observers/merge_request_observer_spec.rb | 48 +------- spec/observers/note_observer_spec.rb | 110 +----------------- spec/observers/user_observer_spec.rb | 8 +- spec/observers/users_project_observer_spec.rb | 5 +- 7 files changed, 19 insertions(+), 164 deletions(-) diff --git a/app/observers/activity_observer.rb b/app/observers/activity_observer.rb index c040c4c5..ee3e4629 100644 --- a/app/observers/activity_observer.rb +++ b/app/observers/activity_observer.rb @@ -1,4 +1,4 @@ -class ActivityObserver < ActiveRecord::Observer +class ActivityObserver < BaseObserver observe :issue, :merge_request, :note, :milestone def after_create(record) diff --git a/app/observers/base_observer.rb b/app/observers/base_observer.rb index b4641bf8..182d3b7b 100644 --- a/app/observers/base_observer.rb +++ b/app/observers/base_observer.rb @@ -1,6 +1,4 @@ class BaseObserver < ActiveRecord::Observer - protected - def notification NotificationService.new end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 05a6730f..37c8345e 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -108,4 +108,12 @@ class NotificationService Notify.delay.note_commit_email(note.commit_author.id, note.id) end end + + def new_team_member(users_project) + Notify.delay.project_access_granted_email(users_project.id) + end + + def update_team_member(users_project) + Notify.delay.project_access_granted_email(users_project.id) + end end diff --git a/spec/observers/merge_request_observer_spec.rb b/spec/observers/merge_request_observer_spec.rb index 991f2d40..2593da72 100644 --- a/spec/observers/merge_request_observer_spec.rb +++ b/spec/observers/merge_request_observer_spec.rb @@ -43,22 +43,21 @@ describe MergeRequestObserver do end end - context 'a reassigned email' do + context 'a notification' do it 'is sent if the merge request is being reassigned' do mr_mock.should_receive(:is_being_reassigned?).and_return(true) - subject.should_receive(:send_reassigned_email).with(mr_mock) + subject.should_receive(:notification) subject.after_update(mr_mock) end it 'is not sent if the merge request is not being reassigned' do mr_mock.should_receive(:is_being_reassigned?).and_return(false) - subject.should_not_receive(:send_reassigned_email) + subject.should_not_receive(:notification) subject.after_update(mr_mock) end end - end context '#after_close' do @@ -92,45 +91,4 @@ describe MergeRequestObserver do end end end - - describe '#send_reassigned_email' do - let(:previous_assignee) { double(:user, id: 3) } - - before(:each) do - mr_mock.stub(:assignee_id).and_return(assignee.id) - mr_mock.stub(:assignee_id_was).and_return(previous_assignee.id) - end - - def it_sends_a_reassigned_email_to(recipient) - Notify.should_receive(:reassigned_merge_request_email).with(recipient, mr_mock.id, previous_assignee.id) - end - - def it_does_not_send_a_reassigned_email_to(recipient) - Notify.should_not_receive(:reassigned_merge_request_email).with(recipient, mr_mock.id, previous_assignee.id) - end - - it 'sends a reassigned email to the previous and current assignees' do - it_sends_a_reassigned_email_to assignee.id - it_sends_a_reassigned_email_to previous_assignee.id - - subject.send(:send_reassigned_email, mr_mock) - end - - context 'does not send an email to the user who made the reassignment' do - it 'if the user is the assignee' do - subject.stub(:current_user).and_return(assignee) - it_sends_a_reassigned_email_to previous_assignee.id - it_does_not_send_a_reassigned_email_to assignee.id - - subject.send(:send_reassigned_email, mr_mock) - end - it 'if the user is the previous assignee' do - subject.stub(:current_user).and_return(previous_assignee) - it_sends_a_reassigned_email_to assignee.id - it_does_not_send_a_reassigned_email_to previous_assignee.id - - subject.send(:send_reassigned_email, mr_mock) - end - end - end end diff --git a/spec/observers/note_observer_spec.rb b/spec/observers/note_observer_spec.rb index 8ad42c21..9ada9270 100644 --- a/spec/observers/note_observer_spec.rb +++ b/spec/observers/note_observer_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe NoteObserver do subject { NoteObserver.instance } + before { subject.stub(notification: mock('NotificationService').as_null_object) } let(:team_without_author) { (1..2).map { |n| double :user, id: n } } @@ -17,116 +18,9 @@ describe NoteObserver do end it 'sends out notifications' do - subject.should_receive(:send_notify_mails).with(note) + subject.should_receive(:notification) subject.after_create(note) end end - - describe "#send_notify_mails" do - let(:note) { double :note, notify: false, notify_author: false } - - it 'notifies team of new note when flagged to notify' do - note.stub(:notify).and_return(true) - subject.should_receive(:notify_team).with(note) - - subject.after_create(note) - end - - it 'does not notify team of new note when not flagged to notify' do - subject.should_not_receive(:notify_team).with(note) - - subject.after_create(note) - end - - it 'notifies the author of a commit when flagged to notify the author' do - note.stub(:notify_author).and_return(true) - note.stub(:noteable).and_return(double(author_email: 'test@test.com')) - note.stub(:id).and_return(42) - author = double :user, id: 1, email: 'test@test.com' - note.stub(:commit_author).and_return(author) - Notify.should_receive(:note_commit_email) - - subject.after_create(note) - end - - it 'does not notify the author of a commit when not flagged to notify the author' do - notify.should_not_receive(:note_commit_email) - - subject.after_create(note) - end - - it 'does nothing if no notify flags are set' do - subject.after_create(note).should be_nil - end - end - - describe '#notify_team' do - let(:note) { double :note, id: 1 } - - before :each do - subject.stub(:team_without_note_author).with(note).and_return(team_without_author) - end - - context 'notifies team of a new note on' do - it 'a commit' do - note.stub(:noteable_type).and_return('Commit') - notify.should_receive(:note_commit_email).twice - - subject.send(:notify_team, note) - end - - it 'an issue' do - note.stub(:noteable_type).and_return('Issue') - notify.should_receive(:note_issue_email).twice - - subject.send(:notify_team, note) - end - - it 'a wiki page' do - note.stub(:noteable_type).and_return('Wiki') - notify.should_receive(:note_wiki_email).twice - - subject.send(:notify_team, note) - end - - it 'a merge request' do - note.stub(:noteable_type).and_return('MergeRequest') - notify.should_receive(:note_merge_request_email).twice - - subject.send(:notify_team, note) - end - - it 'a wall' do - # Note: wall posts have #noteable_type of nil - note.stub(:noteable_type).and_return(nil) - notify.should_receive(:note_wall_email).twice - - subject.send(:notify_team, note) - end - end - - it 'does nothing for a new note on a snippet' do - note.stub(:noteable_type).and_return('Snippet') - - subject.send(:notify_team, note).should be_nil - end - end - - - describe '#team_without_note_author' do - let(:author) { double :user, id: 4 } - - let(:users) { team_without_author + [author] } - let(:project) { double :project, users: users } - let(:note) { double :note, project: project, author: author } - - it 'returns the projects user without the note author included' do - subject.send(:team_without_note_author, note).should == team_without_author - end - end - - def notify - Notify - end end diff --git a/spec/observers/user_observer_spec.rb b/spec/observers/user_observer_spec.rb index b58c5647..5b735a8f 100644 --- a/spec/observers/user_observer_spec.rb +++ b/spec/observers/user_observer_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe UserObserver do subject { UserObserver.instance } + before { subject.stub(notification: mock('NotificationService').as_null_object) } it 'calls #after_create when new users are created' do new_user = build(:user) @@ -11,15 +12,10 @@ describe UserObserver do context 'when a new user is created' do it 'sends an email' do - Notify.should_receive(:new_user_email) + subject.should_receive(:notification) create(:user) end - it 'no email for external' do - Notify.should_not_receive(:new_user_email) - create(:user, extern_uid: '32442eEfsafada') - end - it 'trigger logger' do user = double(:user, id: 42, password: 'P@ssword!', name: 'John', email: 'u@mail.local', extern_uid?: false) Gitlab::AppLogger.should_receive(:info) diff --git a/spec/observers/users_project_observer_spec.rb b/spec/observers/users_project_observer_spec.rb index 068688b0..c034501e 100644 --- a/spec/observers/users_project_observer_spec.rb +++ b/spec/observers/users_project_observer_spec.rb @@ -4,6 +4,7 @@ describe UsersProjectObserver do let(:user) { create(:user) } let(:project) { create(:project) } subject { UsersProjectObserver.instance } + before { subject.stub(notification: mock('NotificationService').as_null_object) } describe "#after_commit" do it "should called when UsersProject created" do @@ -12,7 +13,7 @@ describe UsersProjectObserver do end it "should send email to user" do - Notify.should_receive(:project_access_granted_email).and_return(double(deliver: true)) + subject.should_receive(:notification) Event.stub(:create => true) create(:users_project) @@ -36,7 +37,7 @@ describe UsersProjectObserver do end it "should send email to user" do - Notify.should_receive(:project_access_granted_email) + subject.should_receive(:notification) @users_project.update_attribute(:project_access, UsersProject::MASTER) end