Merge pull request #869 from EFForg/fix_signatures_task

Fix signature cleanup task; add validations to prevent duplicates

Author: esoterik

app/controllers/concerns/tooling.rb

@@ -7,7 +7,7 @@ def create_partner_subscription
     return unless @action_page
     @action_page.partners.each do |partner|
       if params["#{partner.code}_subscribe"] == "1"
-        Subscription.create!(partner_signup_params.merge(partner: partner))
+        Subscription.create(partner_signup_params.merge(partner: partner))
       end
     end
   end

app/models/signature.rb

@@ -13,6 +13,8 @@ class Signature < ActiveRecord::Base
   validates_presence_of :country_code, if: :location_required?
 
   validates :email, email: true
+  validates :email, uniqueness: { scope: :petition_id,
+                                  message: "You've already signed this petition!" }
   validates :zipcode, length: { maximum: 12 }
   validate :country_code, :arbitrary_opinion_of_country_string_validity
 

app/models/subscription.rb

@@ -1,4 +1,5 @@
 class Subscription < ActiveRecord::Base
   belongs_to :partner, counter_cache: true
   validates :email, presence: true, email: true
+  validates :email, uniqueness: { scope: :partner_id }
 end

docker/crontab

@@ -1,4 +1,3 @@
-0 * * * * root su -s/bin/bash www-data -c '. /var/www/.profile && cd /opt/actioncenter && bundle exec rake signatures:deduplicate' >>/proc/1/fd/1 2>&1
 0 0 * * * root su -s/bin/bash www-data -c '. /var/www/.profile && cd /opt/actioncenter && bundle exec rake congress:update'        >>/proc/1/fd/1 2>&1
 0 0 * * * root su -s/bin/bash www-data -c '. /var/www/.profile && cd /opt/actioncenter && bundle exec rake db:sessions:trim'        >>/proc/1/fd/1 2>&1
 

lib/tasks/signatures.rake

@@ -19,16 +19,20 @@ namespace :signatures do
 
   desc "Deduplicate signatures from petitions and subscriptions from partners"
   task deduplicate: :environment do
-    sig_dups = Signature.group(:petition_id, :email).having("count(*) > 1")
-    sig_dups.pluck(:petition_id, :email).each do |petition_id, email|
-      ids = Signature.where(petition_id: petition_id, email: email).order(:created_at).ids
-      Signature.where(id: ids[1..-1]).delete_all
+    sig_dups = Signature.group(:email, :petition_id).count.map do |data, count|
+      data if count > 1
+    end.compact
+    sig_dups.each do |email, petition_id|
+      Signature.where(petition_id: petition_id, email: email)
+        .order(:created_at).drop(1).map(&:delete)
     end
 
-    sub_dups = Subscription.group(:partner_id, :email).having("count(*) > 1")
-    sub_dups.pluck(:partner_id, :email).each do |partner_id, email|
+    sub_dups = Subscription.group(:email, :partner_id).count.map do |data, count|
+      data if count > 1
+    end.compact
+    sub_dups.each do |email, partner_id|
       Subscription.where(partner_id: partner_id, email: email)
-        .order(:created_at)[1..-1].each(&:delete)
+                  .order(:created_at).drop(1).map(&:delete)
     end
   end
 end

spec/controllers/admin/petitions_controller_spec.rb

@@ -34,19 +34,7 @@
   describe "DELETE #destroy_signatures" do
     let(:petition) { FactoryGirl.create(:petition) }
     let(:signatures) do
-      30.times.map do
-        petition.signatures.create(
-          first_name: "Save Kittens",
-          last_name: "Save kittens in great detail",
-          email: "[email protected]",
-          country_code: "US",
-          zipcode: "94109",
-          street_address: "815 Eddy Street",
-          city: "San Francisco",
-          state: "CA",
-          anonymous: false
-        )
-      end
+      30.times.map { FactoryGirl.create(:signature, petition: petition) }
     end
 
     it "should delete signatures from the signature_ids param" do

spec/tasks/signatures_spec.rb

@@ -41,7 +41,8 @@
         FactoryGirl.create(:subscription, partner: partner)
       end
       let!(:dup_subscription) do
-        FactoryGirl.create(:subscription, email: email, partner: partner)
+        FactoryGirl.create(:subscription)
+                   .update_columns(email: email, partner_id: partner.id)
       end
 
       it "removes the newer duplicates" do