refactor observers test since email logic moved to service

Dmitriy Zaporozhets
1 parent ce0945ef
Showing 7 changed files with 19 additions and 164 deletions Show diff stats
app/observers/activity_observer.rb
app/observers/base_observer.rb
app/services/notification_service.rb
spec/observers/merge_request_observer_spec.rb
spec/observers/note_observer_spec.rb
spec/observers/user_observer_spec.rb
spec/observers/users_project_observer_spec.rb
-class ActivityObserver < ActiveRecord::Observer
+class ActivityObserver < BaseObserver
   observe :issue, :merge_request, :note, :milestone
   def after_create(record)
 class BaseObserver < ActiveRecord::Observer
-  protected
-
   def notification
     NotificationService.new
   end
@@ -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
@@ -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
@@ -2,6 +2,7 @@ require &#39;spec_helper&#39;
 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
@@ -2,6 +2,7 @@ require &#39;spec_helper&#39;
 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)
@@ -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
1	-class ActivityObserver < ActiveRecord::Observer	1	+class ActivityObserver < BaseObserver
2	observe :issue, :merge_request, :note, :milestone	2	observe :issue, :merge_request, :note, :milestone
3		3
4	def after_create(record)	4	def after_create(record)
1	class BaseObserver < ActiveRecord::Observer	1	class BaseObserver < ActiveRecord::Observer
2	- protected
3	-
4	def notification	2	def notification
5	NotificationService.new	3	NotificationService.new
6	end	4	end
	@@ -108,4 +108,12 @@ class NotificationService		@@ -108,4 +108,12 @@ class NotificationService
108	Notify.delay.note_commit_email(note.commit_author.id, note.id)	108	Notify.delay.note_commit_email(note.commit_author.id, note.id)
109	end	109	end
110	end	110	end
		111	+
		112	+ def new_team_member(users_project)
		113	+ Notify.delay.project_access_granted_email(users_project.id)
		114	+ end
		115	+
		116	+ def update_team_member(users_project)
		117	+ Notify.delay.project_access_granted_email(users_project.id)
		118	+ end
111	end	119	end
	@@ -43,22 +43,21 @@ describe MergeRequestObserver do		@@ -43,22 +43,21 @@ describe MergeRequestObserver do
43	end	43	end
44	end	44	end
45		45
46	- context 'a reassigned email' do	46	+ context 'a notification' do
47	it 'is sent if the merge request is being reassigned' do	47	it 'is sent if the merge request is being reassigned' do
48	mr_mock.should_receive(:is_being_reassigned?).and_return(true)	48	mr_mock.should_receive(:is_being_reassigned?).and_return(true)
49	- subject.should_receive(:send_reassigned_email).with(mr_mock)	49	+ subject.should_receive(:notification)
50		50
51	subject.after_update(mr_mock)	51	subject.after_update(mr_mock)
52	end	52	end
53		53
54	it 'is not sent if the merge request is not being reassigned' do	54	it 'is not sent if the merge request is not being reassigned' do
55	mr_mock.should_receive(:is_being_reassigned?).and_return(false)	55	mr_mock.should_receive(:is_being_reassigned?).and_return(false)
56	- subject.should_not_receive(:send_reassigned_email)	56	+ subject.should_not_receive(:notification)
57		57
58	subject.after_update(mr_mock)	58	subject.after_update(mr_mock)
59	end	59	end
60	end	60	end
61	-
62	end	61	end
63		62
64	context '#after_close' do	63	context '#after_close' do
	@@ -92,45 +91,4 @@ describe MergeRequestObserver do		@@ -92,45 +91,4 @@ describe MergeRequestObserver do
92	end	91	end
93	end	92	end
94	end	93	end
95	-
96	- describe '#send_reassigned_email' do
97	- let(:previous_assignee) { double(:user, id: 3) }
98	-
99	- before(:each) do
100	- mr_mock.stub(:assignee_id).and_return(assignee.id)
101	- mr_mock.stub(:assignee_id_was).and_return(previous_assignee.id)
102	- end
103	-
104	- def it_sends_a_reassigned_email_to(recipient)
105	- Notify.should_receive(:reassigned_merge_request_email).with(recipient, mr_mock.id, previous_assignee.id)
106	- end
107	-
108	- def it_does_not_send_a_reassigned_email_to(recipient)
109	- Notify.should_not_receive(:reassigned_merge_request_email).with(recipient, mr_mock.id, previous_assignee.id)
110	- end
111	-
112	- it 'sends a reassigned email to the previous and current assignees' do
113	- it_sends_a_reassigned_email_to assignee.id
114	- it_sends_a_reassigned_email_to previous_assignee.id
115	-
116	- subject.send(:send_reassigned_email, mr_mock)
117	- end
118	-
119	- context 'does not send an email to the user who made the reassignment' do
120	- it 'if the user is the assignee' do
121	- subject.stub(:current_user).and_return(assignee)
122	- it_sends_a_reassigned_email_to previous_assignee.id
123	- it_does_not_send_a_reassigned_email_to assignee.id
124	-
125	- subject.send(:send_reassigned_email, mr_mock)
126	- end
127	- it 'if the user is the previous assignee' do
128	- subject.stub(:current_user).and_return(previous_assignee)
129	- it_sends_a_reassigned_email_to assignee.id
130	- it_does_not_send_a_reassigned_email_to previous_assignee.id
131	-
132	- subject.send(:send_reassigned_email, mr_mock)
133	- end
134	- end
135	- end
136	end	94	end
	@@ -2,6 +2,7 @@ require 'spec_helper'		@@ -2,6 +2,7 @@ require 'spec_helper'
2		2
3	describe NoteObserver do	3	describe NoteObserver do
4	subject { NoteObserver.instance }	4	subject { NoteObserver.instance }
		5	+ before { subject.stub(notification: mock('NotificationService').as_null_object) }
5		6
6	let(:team_without_author) { (1..2).map { \|n\| double :user, id: n } }	7	let(:team_without_author) { (1..2).map { \|n\| double :user, id: n } }
7		8
	@@ -17,116 +18,9 @@ describe NoteObserver do		@@ -17,116 +18,9 @@ describe NoteObserver do
17	end	18	end
18		19
19	it 'sends out notifications' do	20	it 'sends out notifications' do
20	- subject.should_receive(:send_notify_mails).with(note)	21	+ subject.should_receive(:notification)
21		22
22	subject.after_create(note)	23	subject.after_create(note)
23	end	24	end
24	end	25	end
25	-
26	- describe "#send_notify_mails" do
27	- let(:note) { double :note, notify: false, notify_author: false }
28	-
29	- it 'notifies team of new note when flagged to notify' do
30	- note.stub(:notify).and_return(true)
31	- subject.should_receive(:notify_team).with(note)
32	-
33	- subject.after_create(note)
34	- end
35	-
36	- it 'does not notify team of new note when not flagged to notify' do
37	- subject.should_not_receive(:notify_team).with(note)
38	-
39	- subject.after_create(note)
40	- end
41	-
42	- it 'notifies the author of a commit when flagged to notify the author' do
43	- note.stub(:notify_author).and_return(true)
44	- note.stub(:noteable).and_return(double(author_email: 'test@test.com'))
45	- note.stub(:id).and_return(42)
46	- author = double :user, id: 1, email: 'test@test.com'
47	- note.stub(:commit_author).and_return(author)
48	- Notify.should_receive(:note_commit_email)
49	-
50	- subject.after_create(note)
51	- end
52	-
53	- it 'does not notify the author of a commit when not flagged to notify the author' do
54	- notify.should_not_receive(:note_commit_email)
55	-
56	- subject.after_create(note)
57	- end
58	-
59	- it 'does nothing if no notify flags are set' do
60	- subject.after_create(note).should be_nil
61	- end
62	- end
63	-
64	- describe '#notify_team' do
65	- let(:note) { double :note, id: 1 }
66	-
67	- before :each do
68	- subject.stub(:team_without_note_author).with(note).and_return(team_without_author)
69	- end
70	-
71	- context 'notifies team of a new note on' do
72	- it 'a commit' do
73	- note.stub(:noteable_type).and_return('Commit')
74	- notify.should_receive(:note_commit_email).twice
75	-
76	- subject.send(:notify_team, note)
77	- end
78	-
79	- it 'an issue' do
80	- note.stub(:noteable_type).and_return('Issue')
81	- notify.should_receive(:note_issue_email).twice
82	-
83	- subject.send(:notify_team, note)
84	- end
85	-
86	- it 'a wiki page' do
87	- note.stub(:noteable_type).and_return('Wiki')
88	- notify.should_receive(:note_wiki_email).twice
89	-
90	- subject.send(:notify_team, note)
91	- end
92	-
93	- it 'a merge request' do
94	- note.stub(:noteable_type).and_return('MergeRequest')
95	- notify.should_receive(:note_merge_request_email).twice
96	-
97	- subject.send(:notify_team, note)
98	- end
99	-
100	- it 'a wall' do
101	- # Note: wall posts have #noteable_type of nil
102	- note.stub(:noteable_type).and_return(nil)
103	- notify.should_receive(:note_wall_email).twice
104	-
105	- subject.send(:notify_team, note)
106	- end
107	- end
108	-
109	- it 'does nothing for a new note on a snippet' do
110	- note.stub(:noteable_type).and_return('Snippet')
111	-
112	- subject.send(:notify_team, note).should be_nil
113	- end
114	- end
115	-
116	-
117	- describe '#team_without_note_author' do
118	- let(:author) { double :user, id: 4 }
119	-
120	- let(:users) { team_without_author + [author] }
121	- let(:project) { double :project, users: users }
122	- let(:note) { double :note, project: project, author: author }
123	-
124	- it 'returns the projects user without the note author included' do
125	- subject.send(:team_without_note_author, note).should == team_without_author
126	- end
127	- end
128	-
129	- def notify
130	- Notify
131	- end
132	end	26	end