Commit d3fc2e6b9f5ae9039869a2b01dee44a1c7e46008
1 parent
3da8f8c0
Exists in
master
and in
28 other branches
Don't abort the batch because of invidual emails
ActionItem2318
Showing
2 changed files
with
31 additions
and
3 deletions
Show diff stats
app/models/mailing.rb
@@ -9,7 +9,11 @@ class Mailing < ActiveRecord::Base | @@ -9,7 +9,11 @@ class Mailing < ActiveRecord::Base | ||
9 | xss_terminate :only => [ :subject, :body ], :with => 'white_list', :on => 'validation' | 9 | xss_terminate :only => [ :subject, :body ], :with => 'white_list', :on => 'validation' |
10 | 10 | ||
11 | after_create do |mailing| | 11 | after_create do |mailing| |
12 | - Delayed::Job.enqueue MailingJob.new(mailing.id) | 12 | + mailing.schedule |
13 | + end | ||
14 | + | ||
15 | + def schedule | ||
16 | + Delayed::Job.enqueue MailingJob.new(self.id) | ||
13 | end | 17 | end |
14 | 18 | ||
15 | def generate_from | 19 | def generate_from |
@@ -30,8 +34,14 @@ class Mailing < ActiveRecord::Base | @@ -30,8 +34,14 @@ class Mailing < ActiveRecord::Base | ||
30 | 34 | ||
31 | def deliver | 35 | def deliver |
32 | each_recipient do |recipient| | 36 | each_recipient do |recipient| |
33 | - Mailing::Sender.deliver_mail(self, recipient.email) | ||
34 | - self.mailing_sents.create(:person => recipient) | 37 | + begin |
38 | + Mailing::Sender.deliver_mail(self, recipient.email) | ||
39 | + self.mailing_sents.create(:person => recipient) | ||
40 | + rescue Exception | ||
41 | + # FIXME should not discard errors silently. An idea is to collect all | ||
42 | + # errors and generate a task (notification) for the +source+ | ||
43 | + # (environment/organization) listing these errors. | ||
44 | + end | ||
35 | end | 45 | end |
36 | end | 46 | end |
37 | 47 |
test/unit/mailing_test.rb
@@ -92,4 +92,22 @@ class MailingTest < ActiveSupport::TestCase | @@ -92,4 +92,22 @@ class MailingTest < ActiveSupport::TestCase | ||
92 | environment.domains << Domain.create(:name => 'noosfero.net', :is_default => true) | 92 | environment.domains << Domain.create(:name => 'noosfero.net', :is_default => true) |
93 | assert_equal '', mailing.url | 93 | assert_equal '', mailing.url |
94 | end | 94 | end |
95 | + | ||
96 | + should 'process the entire batch even if individual emails crash' do | ||
97 | + mailing = Mailing.new(:source => environment, :person => Person['user_one'], :body => 'test', :subject => 'test') | ||
98 | + def mailing.each_recipient | ||
99 | + user_one = Person['user_one'] | ||
100 | + user_two = Person['user_two'] | ||
101 | + user_one.stubs(:email).raises(RuntimeError.new) | ||
102 | + [user_one, user_two].each do |p| | ||
103 | + yield p | ||
104 | + end | ||
105 | + end | ||
106 | + mailing.stubs(:schedule) | ||
107 | + mailing.save! | ||
108 | + mailing.deliver | ||
109 | + | ||
110 | + assert_equal 1, ActionMailer::Base.deliveries.size | ||
111 | + end | ||
112 | + | ||
95 | end | 113 | end |