From 9264763ec5055f5eb75873080a98dfbd75e565bd Mon Sep 17 00:00:00 2001
From: rgarcia <voodoorai2000@gmail.com>
Date: Thu, 17 May 2018 16:53:47 +0200
Subject: [PATCH] Log newsletter emails sent
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

With this commit we are logging which emails have already received the
newsletter

This could be important if something goes wrong sending the newsletter,
to be able to identify which users have already received the newsletter
and be able to skip them

We’ve had to add a new action to the Activity model (email) and add
paranoia features to be able to deal gracefully with the default
`with_hidden` scope in Activities[1]

[1]
https://github.com/AyuntamientoMadrid/consul/blob/master/app/models/acti
vity.rb#L2
---
 app/models/activity.rb                            |  2 +-
 app/models/newsletter.rb                          | 11 ++++++++++-
 ...20180517141120_add_hidden_at_to_newsletters.rb |  5 +++++
 db/schema.rb                                      |  1 +
 spec/models/activity_spec.rb                      |  2 ++
 spec/models/newsletter_spec.rb                    | 15 +++++++++++++++
 6 files changed, 34 insertions(+), 2 deletions(-)
 create mode 100644 db/migrate/20180517141120_add_hidden_at_to_newsletters.rb

diff --git a/app/models/activity.rb b/app/models/activity.rb
index 6c357d891..834f7721c 100644
--- a/app/models/activity.rb
+++ b/app/models/activity.rb
@@ -2,7 +2,7 @@ class Activity < ActiveRecord::Base
   belongs_to :actionable, -> { with_hidden }, polymorphic: true
   belongs_to :user, -> { with_hidden }
 
-  VALID_ACTIONS = %w(hide block restore valuate)
+  VALID_ACTIONS = %w(hide block restore valuate email)
 
   validates :action, inclusion: {in: VALID_ACTIONS}
 
diff --git a/app/models/newsletter.rb b/app/models/newsletter.rb
index e5aac95bf..b99e3e2ca 100644
--- a/app/models/newsletter.rb
+++ b/app/models/newsletter.rb
@@ -8,6 +8,9 @@ class Newsletter < ActiveRecord::Base
 
   validates_format_of :from, :with => /@/
 
+  acts_as_paranoid column: :hidden_at
+  include ActsAsParanoidAliases
+
   def list_of_recipient_emails
     UserSegments.user_segment_emails(segment_recipient) if valid_segment_recipient?
   end
@@ -24,8 +27,9 @@ class Newsletter < ActiveRecord::Base
     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)
+          Mailer.delay(run_at: run_at).newsletter(self, recipient_email)
+          log_delivery(recipient_email)
         end
       end
       run_at += batch_interval
@@ -57,4 +61,9 @@ class Newsletter < ActiveRecord::Base
   def valid_email?(email)
     email.match(/\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i)
   end
+
+  def log_delivery(recipient_email)
+    user = User.where(email: recipient_email).first
+    Activity.log(user, :email, self)
+  end
 end
diff --git a/db/migrate/20180517141120_add_hidden_at_to_newsletters.rb b/db/migrate/20180517141120_add_hidden_at_to_newsletters.rb
new file mode 100644
index 000000000..5e1b4b8cf
--- /dev/null
+++ b/db/migrate/20180517141120_add_hidden_at_to_newsletters.rb
@@ -0,0 +1,5 @@
+class AddHiddenAtToNewsletters < ActiveRecord::Migration
+  def change
+    add_column :newsletters, :hidden_at, :datetime
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 878c16869..142990d93 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -682,6 +682,7 @@ ActiveRecord::Schema.define(version: 20180711224810) do
     t.date     "sent_at"
     t.datetime "created_at",        null: false
     t.datetime "updated_at",        null: false
+    t.datetime "hidden_at"
   end
 
   create_table "notifications", force: :cascade do |t|
diff --git a/spec/models/activity_spec.rb b/spec/models/activity_spec.rb
index 902320ec6..d048a6bb8 100644
--- a/spec/models/activity_spec.rb
+++ b/spec/models/activity_spec.rb
@@ -7,12 +7,14 @@ describe Activity do
     expect(build(:activity, actionable: create(:debate))).to be_valid
     expect(build(:activity, actionable: create(:comment))).to be_valid
     expect(build(:activity, actionable: create(:user))).to be_valid
+    expect(build(:activity, actionable: create(:newsletter))).to be_valid
   end
 
   it "is a valid only with allowed actions" do
     expect(build(:activity, action: "hide")).to be_valid
     expect(build(:activity, action: "block")).to be_valid
     expect(build(:activity, action: "restore")).to be_valid
+    expect(build(:activity, action: "email")).to be_valid
     expect(build(:activity, action: "dissapear")).not_to be_valid
   end
 
diff --git a/spec/models/newsletter_spec.rb b/spec/models/newsletter_spec.rb
index 788326e88..61abb3daf 100644
--- a/spec/models/newsletter_spec.rb
+++ b/spec/models/newsletter_spec.rb
@@ -118,6 +118,21 @@ describe Newsletter do
       expect(Delayed::Job.third.run_at.change(usec: 0)).to eq(third_batch_run_at)
     end
 
+    it "logs users that have received the newsletter" do
+      newsletter.deliver
+
+      expect(Activity.count).to eq(3)
+
+      recipients.each do |email|
+        user = User.where(email: email).first
+        activity = Activity.where(user: user).first
+
+        expect(activity.user_id).to eq(user.id)
+        expect(activity.action).to eq("email")
+        expect(activity.actionable).to eq(newsletter)
+      end
+    end
+
     it "skips invalid emails" do
       Proposal.destroy_all