From 8960928a76becb4429ff1d4a374b26bc66ec437b Mon Sep 17 00:00:00 2001
From: rgarcia <voodoorai2000@gmail.com>
Date: Thu, 17 May 2018 16:48:08 +0200
Subject: [PATCH] Skip invalid emails when sending newsletter

We were seeing an exception when sending some emails due to having
invalid format

With this commit we are skipping this invalid formatted emails when
sending the newsletter
---
 app/models/newsletter.rb       | 32 ++++++++++++++
 spec/models/newsletter_spec.rb | 76 ++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+)

diff --git a/app/models/newsletter.rb b/app/models/newsletter.rb
index 8c3a83cfa..e5aac95bf 100644
--- a/app/models/newsletter.rb
+++ b/app/models/newsletter.rb
@@ -20,9 +20,41 @@ class Newsletter < ActiveRecord::Base
     sent_at.nil?
   end
 
+  def deliver
+    run_at = first_batch_run_at
+    list_of_recipient_emails_in_batches.each do |recipient_emails|
+      recipient_emails.each do |recipient_email|
+        Mailer.delay(run_at: run_at).newsletter(self, recipient_email)
+        if valid_email?(recipient_email)
+        end
+      end
+      run_at += batch_interval
+    end
+  end
+
+  def batch_size
+    10000
+  end
+
+  def batch_interval
+    20.minutes
+  end
+
+  def first_batch_run_at
+    Time.current
+  end
+
+  def list_of_recipient_emails_in_batches
+    list_of_recipient_emails.in_groups_of(batch_size, false)
+  end
+
   private
 
   def validate_segment_recipient
     errors.add(:segment_recipient, :invalid) unless valid_segment_recipient?
   end
+
+  def valid_email?(email)
+    email.match(/\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i)
+  end
 end
diff --git a/spec/models/newsletter_spec.rb b/spec/models/newsletter_spec.rb
index bd42e9177..788326e88 100644
--- a/spec/models/newsletter_spec.rb
+++ b/spec/models/newsletter_spec.rb
@@ -63,4 +63,80 @@ describe Newsletter do
       expect(newsletter.list_of_recipient_emails).not_to include('erased_user@consul.dev')
     end
   end
+
+  describe "#deliver" do
+    let!(:proposals) { Array.new(3) { create(:proposal) } }
+    let!(:debate) { create(:debate) }
+
+    let!(:recipients) { proposals.map(&:author).map(&:email) }
+    let!(:newsletter) { create(:newsletter, segment_recipient: "proposal_authors") }
+
+    before do
+      reset_mailer
+      Delayed::Worker.delay_jobs = true
+    end
+
+    after do
+      Delayed::Worker.delay_jobs = false
+    end
+
+    it "sends an email with the newsletter to every recipient" do
+      newsletter.deliver
+
+      recipients.each do |recipient|
+        email = Mailer.newsletter(newsletter, recipient)
+        expect(email).to deliver_to(recipient)
+      end
+
+      Delayed::Job.all.map(&:invoke_job)
+      expect(ActionMailer::Base.deliveries.count).to eq(3)
+    end
+
+    it "sends emails in batches" do
+      allow(newsletter).to receive(:batch_size).and_return(1)
+
+      newsletter.deliver
+
+      expect(Delayed::Job.count).to eq(3)
+    end
+
+    it "sends batches in time intervals" do
+      allow(newsletter).to receive(:batch_size).and_return(1)
+      allow(newsletter).to receive(:batch_interval).and_return(1.second)
+      allow(newsletter).to receive(:first_batch_run_at).and_return(Time.current)
+
+      newsletter.deliver
+
+      now = newsletter.first_batch_run_at
+      first_batch_run_at  = now.change(usec: 0)
+      second_batch_run_at = (now + 1.second).change(usec: 0)
+      third_batch_run_at  = (now + 2.seconds).change(usec: 0)
+
+      expect(Delayed::Job.count).to eq(3)
+      expect(Delayed::Job.first.run_at.change(usec: 0)).to eq(first_batch_run_at)
+      expect(Delayed::Job.second.run_at.change(usec: 0)).to eq(second_batch_run_at)
+      expect(Delayed::Job.third.run_at.change(usec: 0)).to eq(third_batch_run_at)
+    end
+
+    it "skips invalid emails" do
+      Proposal.destroy_all
+
+      valid_email = "john@gmail.com"
+      invalid_email = "john@gmail..com"
+
+      valid_email_user = create(:user, email: valid_email)
+      proposal = create(:proposal, author: valid_email_user)
+
+      invalid_email_user = create(:user, email: invalid_email)
+      proposal = create(:proposal, author: invalid_email_user)
+
+      newsletter.deliver
+
+      expect(Activity.count).to eq(1)
+      expect(Activity.first.user_id).to eq(valid_email_user.id)
+      expect(Activity.first.action).to eq("email")
+      expect(Activity.first.actionable).to eq(newsletter)
+    end
+
+  end
 end