ActionItem628: extracting method notification_emails

Extracted the logic of getting "who should be notified for a given profile" in its own method. Added a default implementation for Profile and overrided it in Organization to list both the contact_email entered and the administrators e-mails.

ActionItem628: extracting method notification_emails
Extracted the logic of getting "who should be notified for a given profile" in its own method. Added a default implementation for Profile and overrided it in Organization to list both the contact_email entered and the administrators e-mails.
Antonio Terceiro
1 parent e2e0e99d
Showing 8 changed files with 46 additions and 9 deletions Show diff stats
app/models/comment.rb
app/models/contact.rb
app/models/organization.rb
app/models/profile.rb
app/models/task_mailer.rb
test/unit/organization_test.rb
test/unit/profile_test.rb
test/unit/task_mailer_test.rb
@@ -62,8 +62,7 @@ class Comment &lt; ActiveRecord::Base
   class Notifier < ActionMailer::Base
     def mail(comment)
       profile = comment.article.profile
-      # FIXME hardcoded
-      email = profile.person? ? profile.email : profile.contact_email
+      email = profile.notification_emails
       return unless email
       recipients email
@@ -22,7 +22,7 @@ class Contact &lt; ActiveRecord::Base #WithoutTable
   class Sender < ActionMailer::Base
     def mail(contact)
-      emails = [contact.dest.contact_email] + contact.dest.admins.map{|i| i.email}
+      emails = contact.dest.notification_emails
       recipients emails
       from "#{contact.name} <#{contact.email}>"
       if contact.receive_a_copy
@@ -90,4 +90,8 @@ class Organization &lt; Profile
     ]
   end
+  def notification_emails
+    [contact_email].compact + admins.map(&:email)
+  end
+
 end
@@ -233,6 +233,12 @@ class Profile &lt; ActiveRecord::Base
     raise NotImplementedError
   end
+  # This method must return a list of e-mail adresses to which notification messages must be sent.
+  # The implementation in this class just delegates to +contact_email+. Subclasse may override this method.
+  def notification_emails
+    [contact_email]
+  end
+
   # gets recent documents in this profile, ordered from the most recent to the
   # oldest.
   #
@@ -12,7 +12,7 @@ class TaskMailer &lt; ActionMailer::Base
   def target_notification(task, message)
     msg = extract_message(message)
-    recipients task.target.contact_email
+    recipients task.target.notification_emails
     from self.class.generate_from(task)
     subject '[%s] %s' % [task.requestor.environment.name, task.description]
@@ -37,7 +37,7 @@ class TaskMailer &lt; ActionMailer::Base
     text = extract_message(message)
-    recipients task.requestor.email
+    recipients task.requestor.notification_emails
     from self.class.generate_from(task)
     subject '[%s] %s' % [task.requestor.environment.name, task.description]
     body :requestor => task.requestor.name,
@@ -96,6 +96,24 @@ class OrganizationTest &lt; Test::Unit::TestCase
     assert !org.errors.invalid?(:contact_email)
   end
+  should 'list contact_email plus admin emails as "notification emails"' do
+    o = Organization.new(:contact_email => 'org@email.com')
+    admin1 = mock; admin1.stubs(:email).returns('admin1@email.com')
+    admin2 = mock; admin2.stubs(:email).returns('admin2@email.com')
+    o.stubs(:admins).returns([admin1, admin2])
+
+    assert_equal ['org@email.com', 'admin1@email.com', 'admin2@email.com'], o.notification_emails
+  end
+
+  should 'list only admins if contact_email is blank' do
+    o = Organization.new(:contact_email => nil)
+    admin1 = mock; admin1.stubs(:email).returns('admin1@email.com')
+    admin2 = mock; admin2.stubs(:email).returns('admin2@email.com')
+    o.stubs(:admins).returns([admin1, admin2])
+
+    assert_equal ['admin1@email.com', 'admin2@email.com'], o.notification_emails
+  end
+
   should 'list pending enterprise validations' do
     org = Organization.new
     assert_kind_of Array, org.pending_validations
@@ -1139,6 +1139,12 @@ class ProfileTest &lt; Test::Unit::TestCase
     end
   end
+  should 'delegate to contact_email to retrieve notification e-mails' do
+    p = Profile.new
+    p.stubs(:contact_email).returns('my@email.com')
+    assert_equal ['my@email.com'], p.notification_emails
+  end
+
   private
   def assert_invalid_identifier(id)
@@ -23,7 +23,7 @@ class TaskMailerTest &lt; Test::Unit::TestCase
     task.expects(:description).returns('the task')
     requestor = mock()
-    requestor.expects(:email).returns('requestor@example.com')
+    requestor.expects(:notification_emails).returns(['requestor@example.com'])
     requestor.expects(:name).returns('my name')
     environment = mock()
@@ -35,6 +35,7 @@ class TaskMailerTest &lt; Test::Unit::TestCase
     requestor.expects(:environment).returns(environment).at_least_once
     TaskMailer.deliver_task_finished(task)
+    assert !ActionMailer::Base.deliveries.empty?
   end
   should 'be able to send a "task cancelled" message' do
@@ -44,7 +45,7 @@ class TaskMailerTest &lt; Test::Unit::TestCase
     task.expects(:description).returns('the task')
     requestor = mock()
-    requestor.expects(:email).returns('requestor@example.com')
+    requestor.expects(:notification_emails).returns(['requestor@example.com'])
     requestor.expects(:name).returns('my name')
     environment = mock()
@@ -56,6 +57,7 @@ class TaskMailerTest &lt; Test::Unit::TestCase
     requestor.expects(:environment).returns(environment).at_least_once
     TaskMailer.deliver_task_cancelled(task)
+    assert !ActionMailer::Base.deliveries.empty?
   end
   should 'be able to send a "task created" message' do
@@ -66,7 +68,7 @@ class TaskMailerTest &lt; Test::Unit::TestCase
     task.expects(:description).returns('the task')
     requestor = mock()
-    requestor.expects(:email).returns('requestor@example.com')
+    requestor.expects(:notification_emails).returns(['requestor@example.com'])
     requestor.expects(:name).returns('my name')
     environment = mock()
@@ -78,6 +80,7 @@ class TaskMailerTest &lt; Test::Unit::TestCase
     requestor.expects(:environment).returns(environment).at_least_once
     TaskMailer.deliver_task_created(task)
+    assert !ActionMailer::Base.deliveries.empty?
   end
   should 'be able to send a "target notification" message' do
@@ -88,7 +91,7 @@ class TaskMailerTest &lt; Test::Unit::TestCase
     requestor.expects(:name).returns('my name')
     target = mock()
-    target.expects(:contact_email).returns('target@example.com')
+    target.expects(:notification_emails).returns(['target@example.com'])
     target.expects(:name).returns('Target')
     environment = mock()
@@ -101,6 +104,7 @@ class TaskMailerTest &lt; Test::Unit::TestCase
     requestor.expects(:environment).returns(environment).at_least_once
     TaskMailer.deliver_target_notification(task, 'the message')
+    assert !ActionMailer::Base.deliveries.empty?
   end
   should 'use environment name and contact email' do
	@@ -22,7 +22,7 @@ class Contact < ActiveRecord::Base #WithoutTable		@@ -22,7 +22,7 @@ class Contact < ActiveRecord::Base #WithoutTable
22		22
23	class Sender < ActionMailer::Base	23	class Sender < ActionMailer::Base
24	def mail(contact)	24	def mail(contact)
25	- emails = [contact.dest.contact_email] + contact.dest.admins.map{\|i\| i.email}	25	+ emails = contact.dest.notification_emails
26	recipients emails	26	recipients emails
27	from "#{contact.name} <#{contact.email}>"	27	from "#{contact.name} <#{contact.email}>"
28	if contact.receive_a_copy	28	if contact.receive_a_copy
	@@ -90,4 +90,8 @@ class Organization < Profile		@@ -90,4 +90,8 @@ class Organization < Profile
90	]	90	]
91	end	91	end
92		92
		93	+ def notification_emails
		94	+ [contact_email].compact + admins.map(&:email)
		95	+ end
		96	+
93	end	97	end
	@@ -233,6 +233,12 @@ class Profile < ActiveRecord::Base		@@ -233,6 +233,12 @@ class Profile < ActiveRecord::Base
233	raise NotImplementedError	233	raise NotImplementedError
234	end	234	end
235		235
		236	+ # This method must return a list of e-mail adresses to which notification messages must be sent.
		237	+ # The implementation in this class just delegates to +contact_email+. Subclasse may override this method.
		238	+ def notification_emails
		239	+ [contact_email]
		240	+ end
		241	+
236	# gets recent documents in this profile, ordered from the most recent to the	242	# gets recent documents in this profile, ordered from the most recent to the
237	# oldest.	243	# oldest.
238	#	244	#
	@@ -12,7 +12,7 @@ class TaskMailer < ActionMailer::Base		@@ -12,7 +12,7 @@ class TaskMailer < ActionMailer::Base
12	def target_notification(task, message)	12	def target_notification(task, message)
13	msg = extract_message(message)	13	msg = extract_message(message)
14		14
15	- recipients task.target.contact_email	15	+ recipients task.target.notification_emails
16		16
17	from self.class.generate_from(task)	17	from self.class.generate_from(task)
18	subject '[%s] %s' % [task.requestor.environment.name, task.description]	18	subject '[%s] %s' % [task.requestor.environment.name, task.description]
	@@ -37,7 +37,7 @@ class TaskMailer < ActionMailer::Base		@@ -37,7 +37,7 @@ class TaskMailer < ActionMailer::Base
37		37
38	text = extract_message(message)	38	text = extract_message(message)
39		39
40	- recipients task.requestor.email	40	+ recipients task.requestor.notification_emails
41	from self.class.generate_from(task)	41	from self.class.generate_from(task)
42	subject '[%s] %s' % [task.requestor.environment.name, task.description]	42	subject '[%s] %s' % [task.requestor.environment.name, task.description]
43	body :requestor => task.requestor.name,	43	body :requestor => task.requestor.name,
	@@ -96,6 +96,24 @@ class OrganizationTest < Test::Unit::TestCase		@@ -96,6 +96,24 @@ class OrganizationTest < Test::Unit::TestCase
96	assert !org.errors.invalid?(:contact_email)	96	assert !org.errors.invalid?(:contact_email)
97	end	97	end
98		98
		99	+ should 'list contact_email plus admin emails as "notification emails"' do
		100	+ o = Organization.new(:contact_email => 'org@email.com')
		101	+ admin1 = mock; admin1.stubs(:email).returns('admin1@email.com')
		102	+ admin2 = mock; admin2.stubs(:email).returns('admin2@email.com')
		103	+ o.stubs(:admins).returns([admin1, admin2])
		104	+
		105	+ assert_equal ['org@email.com', 'admin1@email.com', 'admin2@email.com'], o.notification_emails
		106	+ end
		107	+
		108	+ should 'list only admins if contact_email is blank' do
		109	+ o = Organization.new(:contact_email => nil)
		110	+ admin1 = mock; admin1.stubs(:email).returns('admin1@email.com')
		111	+ admin2 = mock; admin2.stubs(:email).returns('admin2@email.com')
		112	+ o.stubs(:admins).returns([admin1, admin2])
		113	+
		114	+ assert_equal ['admin1@email.com', 'admin2@email.com'], o.notification_emails
		115	+ end
		116	+
99	should 'list pending enterprise validations' do	117	should 'list pending enterprise validations' do
100	org = Organization.new	118	org = Organization.new
101	assert_kind_of Array, org.pending_validations	119	assert_kind_of Array, org.pending_validations
	@@ -1139,6 +1139,12 @@ class ProfileTest < Test::Unit::TestCase		@@ -1139,6 +1139,12 @@ class ProfileTest < Test::Unit::TestCase
1139	end	1139	end
1140	end	1140	end
1141		1141
		1142	+ should 'delegate to contact_email to retrieve notification e-mails' do
		1143	+ p = Profile.new
		1144	+ p.stubs(:contact_email).returns('my@email.com')
		1145	+ assert_equal ['my@email.com'], p.notification_emails
		1146	+ end
		1147	+
1142	private	1148	private
1143		1149
1144	def assert_invalid_identifier(id)	1150	def assert_invalid_identifier(id)