From d3fc2e6b9f5ae9039869a2b01dee44a1c7e46008 Mon Sep 17 00:00:00 2001
From: Antonio Terceiro <terceiro@colivre.coop.br>
Date: Fri, 10 Aug 2012 11:02:53 -0300
Subject: [PATCH] Don't abort the batch because of invidual emails

---
 app/models/mailing.rb     | 16 +++++++++++++---
 test/unit/mailing_test.rb | 18 ++++++++++++++++++
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/app/models/mailing.rb b/app/models/mailing.rb
index 29ea1bd..2ad69d1 100644
--- a/app/models/mailing.rb
+++ b/app/models/mailing.rb
@@ -9,7 +9,11 @@ class Mailing < ActiveRecord::Base
   xss_terminate :only => [ :subject, :body ], :with => 'white_list', :on => 'validation'
 
   after_create do |mailing|
-    Delayed::Job.enqueue MailingJob.new(mailing.id)
+    mailing.schedule
+  end
+
+  def schedule
+    Delayed::Job.enqueue MailingJob.new(self.id)
   end
 
   def generate_from
@@ -30,8 +34,14 @@ class Mailing < ActiveRecord::Base
 
   def deliver
     each_recipient do |recipient|
-      Mailing::Sender.deliver_mail(self, recipient.email)
-      self.mailing_sents.create(:person => recipient)
+      begin
+        Mailing::Sender.deliver_mail(self, recipient.email)
+        self.mailing_sents.create(:person => recipient)
+      rescue Exception
+        # FIXME should not discard errors silently. An idea is to collect all
+        # errors and generate a task (notification) for the +source+
+        # (environment/organization) listing these errors.
+      end
     end
   end
 
diff --git a/test/unit/mailing_test.rb b/test/unit/mailing_test.rb
index afa1537..331e28f 100644
--- a/test/unit/mailing_test.rb
+++ b/test/unit/mailing_test.rb
@@ -92,4 +92,22 @@ class MailingTest < ActiveSupport::TestCase
     environment.domains << Domain.create(:name => 'noosfero.net', :is_default => true)
     assert_equal '', mailing.url
   end
+
+  should 'process the entire batch even if individual emails crash' do
+    mailing = Mailing.new(:source => environment, :person => Person['user_one'], :body => 'test', :subject => 'test')
+    def mailing.each_recipient
+      user_one = Person['user_one']
+      user_two = Person['user_two']
+      user_one.stubs(:email).raises(RuntimeError.new)
+      [user_one, user_two].each do |p|
+        yield p
+      end
+    end
+    mailing.stubs(:schedule)
+    mailing.save!
+    mailing.deliver
+
+    assert_equal 1, ActionMailer::Base.deliveries.size
+  end
+
 end
--
libgit2 0.21.2