Commit e6b525e9d27b51922ddceb7b6ca50a5cd981af0e
1 parent
93103d85
Exists in
master
and in
22 other branches
Checks whether there is target email before sending notification
If ActionMailer tries to send a mail without recipient, the smtp will throw an syntax error. This checks whether there is a target and that given target has an email for notification before calling the TaskMailer to deliver the message. Also fixes the tests that crashed because of this new restriction and creates a test to this restriction. (ActionItem2370)
Showing
4 changed files
with
27 additions
and
7 deletions
Show diff stats
app/models/task.rb
... | ... | @@ -4,7 +4,7 @@ |
4 | 4 | # |
5 | 5 | # The specific types of tasks <em>must</em> override the #perform method, so |
6 | 6 | # the actual action associated to the type of task can be performed. See the |
7 | -# documentation of the #perform method for details. | |
7 | +# documentation of the #perform method for details. | |
8 | 8 | # |
9 | 9 | # This class has a +data+ field of type <tt>text</tt>, where you can store any |
10 | 10 | # type of data (as serialized Ruby objects) you need for your subclass (which |
... | ... | @@ -64,7 +64,8 @@ class Task < ActiveRecord::Base |
64 | 64 | |
65 | 65 | begin |
66 | 66 | target_msg = task.target_notification_message |
67 | - TaskMailer.deliver_target_notification(task, target_msg) if target_msg | |
67 | + target_emails = task.target && task.target.notification_emails || [] | |
68 | + TaskMailer.deliver_target_notification(task, target_msg) if target_msg && !target_emails.empty? | |
68 | 69 | rescue NotImplementedError => ex |
69 | 70 | RAILS_DEFAULT_LOGGER.info ex.to_s |
70 | 71 | end |
... | ... | @@ -192,7 +193,7 @@ class Task < ActiveRecord::Base |
192 | 193 | |
193 | 194 | # The message that will be sent to the *target* of the task when it is |
194 | 195 | # created. The indent of this message is to notify the target about the |
195 | - # request that was just created for him/her. | |
196 | + # request that was just created for him/her. | |
196 | 197 | # |
197 | 198 | # The implementation in this class returns +nil+, what makes the notification |
198 | 199 | # not to be sent. If you want to send a notification to the target upon task |
... | ... | @@ -225,7 +226,8 @@ class Task < ActiveRecord::Base |
225 | 226 | |
226 | 227 | begin |
227 | 228 | target_msg = target_notification_message |
228 | - TaskMailer.deliver_target_notification(self, target_msg) if target_msg | |
229 | + target_emails = self.target && self.target.notification_emails || [] | |
230 | + TaskMailer.deliver_target_notification(self, target_msg) if target_msg && !target_emails.empty? | |
229 | 231 | rescue NotImplementedError => ex |
230 | 232 | RAILS_DEFAULT_LOGGER.info ex.to_s |
231 | 233 | end |
... | ... | @@ -253,7 +255,7 @@ class Task < ActiveRecord::Base |
253 | 255 | |
254 | 256 | # sends notification e-mail about a task, if the task has a requestor. |
255 | 257 | # |
256 | - # If | |
258 | + # If | |
257 | 259 | def send_notification(action) |
258 | 260 | if sends_email? |
259 | 261 | if self.requestor | ... | ... |
test/unit/add_member_test.rb
... | ... | @@ -54,6 +54,7 @@ class AddMemberTest < ActiveSupport::TestCase |
54 | 54 | |
55 | 55 | should 'send e-mails' do |
56 | 56 | community.update_attribute(:closed, true) |
57 | + community.stubs(:notification_emails).returns(["adm@example.com"]) | |
57 | 58 | |
58 | 59 | TaskMailer.expects(:deliver_target_notification).at_least_once |
59 | 60 | ... | ... |
test/unit/approve_article_test.rb
... | ... | @@ -82,6 +82,7 @@ class ApproveArticleTest < ActiveSupport::TestCase |
82 | 82 | should 'notify target if group is moderated' do |
83 | 83 | community.moderated_articles = true |
84 | 84 | community.save |
85 | + community.stubs(:notification_emails).returns(['adm@example.com']) | |
85 | 86 | |
86 | 87 | a = ApproveArticle.create!(:name => '', :article => article, :target => community, :requestor => profile) |
87 | 88 | assert !ActionMailer::Base.deliveries.empty? | ... | ... |
test/unit/task_test.rb
... | ... | @@ -128,7 +128,7 @@ class TaskTest < ActiveSupport::TestCase |
128 | 128 | task = Task.new |
129 | 129 | task.requestor = sample_user |
130 | 130 | task.save! |
131 | - | |
131 | + | |
132 | 132 | task.cancel |
133 | 133 | |
134 | 134 | assert_nil Task.find_by_code(task.code) |
... | ... | @@ -160,6 +160,9 @@ class TaskTest < ActiveSupport::TestCase |
160 | 160 | |
161 | 161 | should 'send notification to target just after task creation' do |
162 | 162 | task = Task.new |
163 | + target = Profile.new | |
164 | + target.stubs(:notification_emails).returns(['adm@example.com']) | |
165 | + task.target = target | |
163 | 166 | task.stubs(:target_notification_message).returns('some non nil message to be sent to target') |
164 | 167 | TaskMailer.expects(:deliver_target_notification).once |
165 | 168 | task.save! |
... | ... | @@ -204,7 +207,7 @@ class TaskTest < ActiveSupport::TestCase |
204 | 207 | user.destroy |
205 | 208 | end |
206 | 209 | end |
207 | - | |
210 | + | |
208 | 211 | should 'not deliver notification message to target' do |
209 | 212 | task = Task.new |
210 | 213 | assert_raise NotImplementedError do |
... | ... | @@ -228,6 +231,16 @@ class TaskTest < ActiveSupport::TestCase |
228 | 231 | task.save! |
229 | 232 | end |
230 | 233 | |
234 | + should 'not notify target if notification emails is empty' do | |
235 | + task = Task.new | |
236 | + target = Profile.new | |
237 | + target.stubs(:notification_emails).returns([]) | |
238 | + task.target = target | |
239 | + task.stubs(:target_notification_message).returns('some non nil message to be sent to target') | |
240 | + TaskMailer.expects(:deliver_target_notification).never | |
241 | + task.save! | |
242 | + end | |
243 | + | |
231 | 244 | should 'the environment method be defined' do |
232 | 245 | task = Task.new |
233 | 246 | assert task.method_exists?('environment') |
... | ... | @@ -266,6 +279,9 @@ class TaskTest < ActiveSupport::TestCase |
266 | 279 | |
267 | 280 | should 'send notification message to target just after task activation' do |
268 | 281 | task = Task.new(:status => Task::Status::HIDDEN) |
282 | + target = Profile.new | |
283 | + target.stubs(:notification_emails).returns(['target@example.com']) | |
284 | + task.target = target | |
269 | 285 | task.save! |
270 | 286 | task.stubs(:target_notification_message).returns('some non nil message to be sent to target') |
271 | 287 | TaskMailer.expects(:deliver_target_notification).once | ... | ... |