From e59d9cbe7a2c5ad5a251c5c4a8104dad43324789 Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sat, 5 Apr 2014 18:52:12 +0000 Subject: [PATCH] VRFS-1483 email error handling --- admin/Gemfile | 1 + admin/app/admin/email_batch.rb | 69 ++++++++++--------- admin/app/admin/email_error_batch.rb | 33 +++++++++ db/up/email_batch.sql | 29 ++------ db/up/emails.sql | 21 ++---- ruby/Gemfile | 2 +- ruby/lib/jam_ruby.rb | 1 + ruby/lib/jam_ruby/models/email_batch.rb | 76 +++++++++++++++++---- ruby/lib/jam_ruby/models/email_batch_set.rb | 28 +++----- ruby/lib/jam_ruby/models/email_error.rb | 75 +++++++++++++++++++- web/Gemfile | 2 +- websocket-gateway/Gemfile | 2 + 12 files changed, 232 insertions(+), 107 deletions(-) create mode 100644 admin/app/admin/email_error_batch.rb diff --git a/admin/Gemfile b/admin/Gemfile index 5fea93d71..b121b5991 100644 --- a/admin/Gemfile +++ b/admin/Gemfile @@ -70,6 +70,7 @@ gem 'sendgrid', '1.2.0' gem 'geokit-rails' gem 'postgres_ext', '1.0.0' gem 'resque_mailer' +gem 'rest-client' group :libv8 do gem 'libv8', "~> 3.11.8" diff --git a/admin/app/admin/email_batch.rb b/admin/app/admin/email_batch.rb index 3f15aadd3..55e309d85 100644 --- a/admin/app/admin/email_batch.rb +++ b/admin/app/admin/email_batch.rb @@ -1,6 +1,6 @@ ActiveAdmin.register JamRuby::EmailBatch, :as => 'Batch Emails' do - menu :label => 'Emails' + menu :label => 'Batch Emails', :parent => 'Email' config.sort_order = 'updated_at DESC' config.batch_actions = false @@ -10,40 +10,43 @@ ActiveAdmin.register JamRuby::EmailBatch, :as => 'Batch Emails' do form :partial => 'form' index do - column 'Subject' do |pp| pp.subject end - column 'Updated' do |pp| pp.updated_at end - column 'From' do |pp| pp.from_email end - column 'Status' do |pp| pp.aasm_state end - column 'Test Emails' do |pp| pp.test_emails end - column 'Email Count' do |pp| pp.qualified_count end - column 'Send Count' do |pp| pp.sent_count end - column 'Started' do |pp| pp.started_at end - column 'Completed' do |pp| pp.completed_at end - column 'Send Test' do |pp| - link_to("Test Batch (#{pp.test_count})", - batch_test_admin_batch_email_path(pp.id), - :confirm => "Run test batch with #{pp.test_count} emails?") + column 'Subject' do |bb| bb.subject end + column 'Updated' do |bb| bb.updated_at end + column 'From' do |bb| bb.from_email end + column 'Status' do |bb| bb.aasm_state end + column 'Test Emails' do |bb| bb.test_emails end + column 'Email Count' do |bb| bb.candidate_count end + column 'Sent Count' do |bb| bb.sent_count end + column 'Started' do |bb| bb.started_at end + column 'Completed' do |bb| bb.completed_at end + column 'Send Test' do |bb| + bb.can_run_test? ? link_to("Test Batch (#{bb.test_count})", + batch_test_admin_batch_email_path(bb.id), + :confirm => "Run test batch with #{bb.test_count} emails?") : '' end - column 'Send Live' do |pp| - link_to("Live Batch (#{User.email_opt_in.count})", - batch_send_admin_batch_email_path(pp.id), - :confirm => "Run LIVE batch with #{User.email_opt_in.count} emails?") + column 'Send Live' do |bb| + bb.can_run_batch? ? link_to("Live Batch (#{User.email_opt_in.count})", + batch_send_admin_batch_email_path(bb.id), + :confirm => "Run LIVE batch with #{User.email_opt_in.count} emails?") : '' + end + column 'Clone' do |bb| + link_to("Clone", batch_clone_admin_batch_email_path(bb.id)) end default_actions end - action_item :only => :show do - link_to("Send Test Batch (#{resource.test_count})", - batch_test_admin_batch_email_path(resource.id), - :confirm => "Run test batch with #{resource.test_count} emails?") - end + # action_item :only => :show do + # link_to("Send Test Batch (#{resource.test_count})", + # batch_test_admin_batch_email_path(resource.id), + # :confirm => "Run test batch with #{resource.test_count} emails?") + # end - action_item :only => :show do - link_to("Send Live Batch (#{User.email_opt_in.count})", - batch_send_admin_batch_email_path(resource.id), - :confirm => "Run LIVE batch with #{User.email_opt_in.count} emails?") - end + # action_item :only => :show do + # link_to("Send Live Batch (#{User.email_opt_in.count})", + # batch_send_admin_batch_email_path(resource.id), + # :confirm => "Run LIVE batch with #{User.email_opt_in.count} emails?") + # end show :title => 'Batch Email' do |obj| panel 'Email Contents' do @@ -59,9 +62,7 @@ ActiveAdmin.register JamRuby::EmailBatch, :as => 'Batch Emails' do panel 'Sending Parameters' do attributes_table_for obj do row 'State' do |obj| obj.aasm_state end - row 'User Count' do |obj| - obj.qualified_count ? obj.qualified_count : User.email_opt_in.count - end + row 'Opt-in Count' do |obj| obj.opting_in_count end row 'Sent Count' do |obj| obj.sent_count end row 'Started' do |obj| obj.started_at end row 'Completed' do |obj| obj.completed_at end @@ -103,4 +104,10 @@ ActiveAdmin.register JamRuby::EmailBatch, :as => 'Batch Emails' do redirect_to admin_batch_email_path(batch.id) end + member_action :batch_clone, :method => :get do + batch = EmailBatch.find(params[:id]) + batch.clone + redirect_to edit_admin_batch_email_path(batch.id) + end + end diff --git a/admin/app/admin/email_error_batch.rb b/admin/app/admin/email_error_batch.rb new file mode 100644 index 000000000..c41b7373a --- /dev/null +++ b/admin/app/admin/email_error_batch.rb @@ -0,0 +1,33 @@ +ActiveAdmin.register JamRuby::EmailError, :as => 'Batch Email Errors' do + + menu :label => 'Batch Errors', :parent => 'Email' + + config.batch_actions = false + config.filters = false + config.clear_action_items! + + index do + column 'Batch' do |eerr| + link_to(truncate(eerr.batch_subject, :length => 40), admin_batch_email_path(eerr.email_batch_id)) + end + column 'User' do |eerr| + eerr.user ? link_to(eerr.user.name, batch_action_admin_users_path(eerr.user_id)) : 'N/A' + end + column 'Error Type' do |eerr| eerr.error_type end + column 'Email Address' do |eerr| eerr.email_address end + column 'Status' do |eerr| eerr.status end + column 'Reason' do |eerr| eerr.reason end + column 'Email Date' do |eerr| eerr.email_date end + end + + controller do + + def scoped_collection + @eerrors ||= end_of_association_chain + .where(['email_batch_id IS NOT NULL']) + .includes([:user, :email_batch]) + .order('email_date DESC') + end + end + +end diff --git a/db/up/email_batch.sql b/db/up/email_batch.sql index d10ea74bc..b4039a6f6 100644 --- a/db/up/email_batch.sql +++ b/db/up/email_batch.sql @@ -1,31 +1,10 @@ --- CREATE TABLE email_batches ( --- id VARCHAR(64) PRIMARY KEY DEFAULT uuid_generate_v4(), --- subject VARCHAR(256) NOT NULL, --- body TEXT NOT NULL, --- from_email VARCHAR(64) NOT NULL default 'support@jamkazam.com', - --- aasm_state VARCHAR(32) NOT NULL default 'pending', - --- test_emails TEXT NOT NULL default '', - --- qualified_count INTEGER NOT NULL default 0, --- sent_count INTEGER NOT NULL default 0, - --- lock_version INTEGER, - --- started_at TIMESTAMP, --- completed_at TIMESTAMP, - --- created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, --- updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP --- ); - CREATE TABLE email_batch_sets ( id VARCHAR(64) PRIMARY KEY DEFAULT uuid_generate_v4(), email_batch_id VARCHAR(64) REFERENCES email_batches(id) ON DELETE CASCADE, started_at TIMESTAMP, user_ids TEXT NOT NULL default '', + batch_count INTEGER, created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP @@ -40,9 +19,13 @@ CREATE TABLE email_errors ( error_type VARCHAR(32), email_address VARCHAR(256), + status VARCHAR(32), + email_date TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + reason TEXT, created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ); -CREATE INDEX email_error_fkidx ON email_errors(email_batch_id); +CREATE INDEX email_error_batch_fkidx ON email_errors(email_batch_id); +CREATE INDEX email_error_user_fkidx ON email_errors(user_id); diff --git a/db/up/emails.sql b/db/up/emails.sql index 86f8b4f6f..cf76242d2 100644 --- a/db/up/emails.sql +++ b/db/up/emails.sql @@ -6,9 +6,9 @@ CREATE TABLE email_batches ( aasm_state VARCHAR(32) NOT NULL default 'pending', - test_emails TEXT NOT NULL default '', + test_emails TEXT NOT NULL default 'test@jamkazam.com', - qualified_count INTEGER NOT NULL default 0, + candidate_count INTEGER NOT NULL default 0, sent_count INTEGER NOT NULL default 0, lock_version INTEGER, @@ -20,19 +20,6 @@ CREATE TABLE email_batches ( updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ); --- CREATE TABLE email_batch_results ( --- id VARCHAR(64) PRIMARY KEY DEFAULT uuid_generate_v4(), --- email_batch_id VARCHAR(64) REFERENCES email_batches(id) ON DELETE CASCADE, --- user_id VARCHAR(64) REFERENCES users(id) ON DELETE CASCADE, - --- error_type VARCHAR(32), --- email_address VARCHAR(256), - --- created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, --- updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP --- ); - --- ALTER TABLE email_batch_results ADD CONSTRAINT email_batch_uniqkey UNIQUE (email_batch_id); --- ALTER TABLE email_batch_results ADD CONSTRAINT email_user_uniqkey UNIQUE (user_id); - ALTER TABLE users ALTER COLUMN subscribe_email SET DEFAULT true; +UPDATE users SET subscribe_email = true WHERE subscribe_email = false; + diff --git a/ruby/Gemfile b/ruby/Gemfile index b9ebfbded..f81545af0 100644 --- a/ruby/Gemfile +++ b/ruby/Gemfile @@ -44,6 +44,7 @@ gem 'resque_mailer' gem 'oj' gem 'builder' gem 'fog' +gem 'rest-client' group :test do gem 'simplecov', '~> 0.7.1' @@ -52,7 +53,6 @@ group :test do gem "rspec", "2.11" gem 'spork', '0.9.0' gem 'database_cleaner', '0.7.0' - gem 'rest-client' gem 'faker' gem 'resque_spec' end diff --git a/ruby/lib/jam_ruby.rb b/ruby/lib/jam_ruby.rb index 2bc1972a9..d6d49ac15 100755 --- a/ruby/lib/jam_ruby.rb +++ b/ruby/lib/jam_ruby.rb @@ -17,6 +17,7 @@ require "postgres_ext" require 'builder' require 'cgi' require 'resque_mailer' +require 'rest-client' require "jam_ruby/constants/limits" require "jam_ruby/constants/notification_types" diff --git a/ruby/lib/jam_ruby/models/email_batch.rb b/ruby/lib/jam_ruby/models/email_batch.rb index a7e2ae9df..d42139051 100644 --- a/ruby/lib/jam_ruby/models/email_batch.rb +++ b/ruby/lib/jam_ruby/models/email_batch.rb @@ -2,23 +2,26 @@ module JamRuby class EmailBatch < ActiveRecord::Base self.table_name = "email_batches" - has_many :email_batch_sets, :class_name => 'JamRuby::EmailBatchSet', :dependent => :destroy + has_many :email_batch_sets, :class_name => 'JamRuby::EmailBatchSet' has_many :email_errors, :class_name => 'JamRuby::EmailError' attr_accessible :from_email, :subject, :test_emails, :body - attr_accessible :lock_version, :qualified_count, :sent_count, :started_at, :completed_at + attr_accessible :lock_version, :candidate_count, :sent_count, :started_at, :completed_at + + default_scope :order => 'updated_at DESC' VAR_FIRST_NAME = '@FIRSTNAME' VAR_LAST_NAME = '@LASTNAME' DEFAULT_SENDER = "support@jamkazam.com" + BATCH_SIZE = 1000 BODY_TEMPLATE =<Pellentesque facilisis metus ac cursus varius. Nunc laoreet diam mauris, et rhoncus quam commodo vel. Vestibulum nec diam lobortis, posuere sapien id, faucibus nulla. Vivamus vitae pellentesque massa. Proin quis nibh eu nibh imperdiet porttitor.

+

Paragraph 1 ... newline whitespace is significant for plain text conversions

-

Vestibulum mollis enim eu fringilla vulputate. Nam tincidunt, enim eget fringilla blandit, mi neque dictum dolor, non pellentesque libero erat sed massa. Morbi sodales lobortis eros, sed feugiat eros euismod eget. Nulla vulputate lobortis porttitor.

+

Paragraph 2 ... #{VAR_FIRST_NAME} will be replaced by users first name

Thanks for using JamKazam!

@@ -32,13 +35,15 @@ FOO state :tested state :batching state :batched + state :confirming + state :confirmed state :disabled event :enable do transitions :from => :disabled, :to => :pending end event :reset do - transitions :from => [:disabled, :testing, :tested, :batching, :batched, :pending], :to => :pending + transitions :from => [:confirming, :confirmed, :disabled, :testing, :tested, :batching, :batched, :pending], :to => :pending end event :do_test_run, :before => :running_tests do transitions :from => [:pending, :tested, :batched], :to => :testing @@ -55,21 +60,40 @@ FOO event :disable do transitions :from => [:pending, :tested, :batched], :to => :disabled end + event :do_confirmation do + transitions :from => [:batched, :tested], :to => :confirming + end + event :did_confirmation do + transitions :from => [:confirming], :to => :confirmed + end + end + + def self.new(*args) + oo = super + oo.body = BODY_TEMPLATE + oo.test_emails = "test@jamkazam.com, test@example.com" + oo end def self.create_with_params(params) obj = self.new params.each { |kk,vv| vv.strip! } - params[:body] = BODY_TEMPLATE if params[:body].empty? obj.update_with_conflict_validation(params) obj end + def can_run_batch? + self.tested? || self.pending? + end + + def can_run_test? + self.test_emails.present? && (self.tested? || self.pending?) + end + def deliver_batch self.perform_event('do_batch_run!') - User.email_opt_in.find_in_batches(batch_size: 1000) do |users| - self.email_batch_sets << EmailBatchSet.deliver_set(self, users.map(&:id)) - # BatchMailer.send_batch_email(self.id, users.map(&:id)).deliver + User.email_opt_in.find_in_batches(batch_size: BATCH_SIZE) do |users| + self.email_batch_sets << EmailBatchSet.deliver_set(self.id, users.map(&:id)) end end @@ -90,7 +114,11 @@ FOO def send_test_batch self.perform_event('do_test_run!') - self.email_batch_sets << EmailBatchSet.deliver_test(self) + if 'test'==Rails.env + BatchMailer.send_batch_email_test(batch.id).deliver! + else + BatchMailer.send_batch_email_test(batch.id).deliver + end end def merged_body(user) @@ -100,7 +128,7 @@ FOO def did_send(emails) self.update_with_conflict_validation({ :sent_count => self.sent_count + emails.size }) - if self.sent_count >= self.qualified_count + if self.sent_count >= self.candidate_count if batching? self.perform_event('did_batch_run!') elsif testing? @@ -132,14 +160,14 @@ FOO end def running_batch - self.update_with_conflict_validation({:qualified_count => User.email_opt_in.count, + self.update_with_conflict_validation({:candidate_count => User.email_opt_in.count, :sent_count => 0, :started_at => Time.now }) end def running_tests - self.update_with_conflict_validation({:qualified_count => self.test_count, + self.update_with_conflict_validation({:candidate_count => self.test_count, :sent_count => 0, :started_at => Time.now }) @@ -147,11 +175,31 @@ FOO def ran_tests self.update_with_conflict_validation({ :completed_at => Time.now }) + perform_confirmation end def ran_batch self.update_with_conflict_validation({ :completed_at => Time.now }) - + perform_confirmation + end + + def perform_confirmation + self.perform_event('do_confirmation!') + EmailError.confirm_errors(self) + end + + def clone + bb = EmailBatch.new + bb.subject = self.subject + bb.body = self.body + bb.from_email = self.from_email + bb.test_emails = self.test_emails + bb.save! + bb + end + + def opting_in_count + 0 < candidate_count ? candidate_count : User.email_opt_in.count end end diff --git a/ruby/lib/jam_ruby/models/email_batch_set.rb b/ruby/lib/jam_ruby/models/email_batch_set.rb index 1e2078b75..14f201333 100644 --- a/ruby/lib/jam_ruby/models/email_batch_set.rb +++ b/ruby/lib/jam_ruby/models/email_batch_set.rb @@ -4,32 +4,22 @@ module JamRuby belongs_to :email_batch, :class_name => 'JamRuby::EmailBatch' - def self.deliver_set(batch, user_ids) + BATCH_SIZE = 1000 + + def self.deliver_set(batch_id, user_ids) bset = self.new - bset.email_batch = batch + bset.email_batch_id = batch_id bset.user_ids = user_ids.join(',') bset.started_at = Time.now + bset.batch_count = user_ids.size bset.save! - if 'test'==Rails.env - BatchMailer.send_batch_email(self.email_batch_id, user_ids).deliver! + + if 'test' == Rails.env + BatchMailer.send_batch_email(bset.email_batch_id, user_ids).deliver! else - BatchMailer.send_batch_email(self.email_batch_id, user_ids).deliver + BatchMailer.send_batch_email(bset.email_batch_id, user_ids).deliver end bset end - - def self.deliver_test(batch) - bset = self.new - bset.email_batch = batch - bset.started_at = Time.now - bset.save! - if 'test'==Rails.env - BatchMailer.send_batch_email_test(batch.id).deliver! - else - BatchMailer.send_batch_email_test(batch.id).deliver - end - bset - end - end end diff --git a/ruby/lib/jam_ruby/models/email_error.rb b/ruby/lib/jam_ruby/models/email_error.rb index 6c4b7b4e3..24b382d5a 100644 --- a/ruby/lib/jam_ruby/models/email_error.rb +++ b/ruby/lib/jam_ruby/models/email_error.rb @@ -1,11 +1,84 @@ module JamRuby class EmailError < ActiveRecord::Base - self.table_name = "email_batch_errors" + self.table_name = "email_errors" belongs_to :email_batch, :class_name => 'JamRuby::EmailBatch' + belongs_to :user, :class_name => 'JamRuby::User' + + default_scope :order => 'email_date DESC' ERR_BOUNCE = :bounce ERR_INVALID = :invalid + SENDGRID_UNAME = 'jamkazam' + SENDGRID_PASSWD = 'jamjamblueberryjam' + + def self.sendgrid_url(resource, action='get', params='') + "https://api.sendgrid.com/api/#{resource}.#{action}.json?api_user=#{EmailError::SENDGRID_UNAME}&api_key=#{EmailError::SENDGRID_PASSWD}&date=1&#{params}" + end + + def self.bounce_url(batch) + uu = sendgrid_url('bounces') + uu += "&start_date=#{batch.started_at.strftime('%Y-%m-%d')}&end_date=#{batch.completed_at.strftime('%Y-%m-%d')}" if batch.batched? + uu + end + + def self.bounce_errors(batch) + uu = self.bounce_url(batch) + response = RestClient.get(uu) + if 200 == response.code + return JSON.parse(response.body).collect do |jj| + ee = EmailError.new + ee.error_type = 'bounces' + ee.email_batch_id = batch.id + ee.email_address = jj['email'] + ee.user_id = User.where(:email => ee.email_address).pluck(:id).first + ee.status = jj['status'] + ee.email_date = jj['created'] + ee.reason = jj['reason'] + ee.save! + RestClient.delete(self.sendgrid_url('bounces', 'delete', "email=#{ee.email_address}")) + ee + end + end + end + + def self.invalid_url(batch) + uu = sendgrid_url('invalidemails') + uu += "&start_date=#{batch.started_at.strftime('%Y-%m-%d')}&end_date=#{batch.completed_at.strftime('%Y-%m-%d')}" if batch.batched? + uu + end + + def self.invalid_errors(batch) + uu = self.invalid_url(batch) + response = RestClient.get(uu) + if 200 == response.code + return JSON.parse(response.body).collect do |jj| + ee = EmailError.new + ee.error_type = 'invalidemails' + ee.email_batch_id = batch.id + ee.email_address = jj['email'] + ee.user_id = User.where(:email => ee.email_address).pluck(:id).first + ee.email_date = jj['created'] + ee.reason = jj['reason'] + ee.save! + uu = + RestClient.delete(self.sendgrid_url('invalidemails', 'delete', "email=#{ee.email_address}")) + ee + end + end + end + + def self.collect_errors(batch) + if batch.batched? + EmailError.bounce_errors(batch) + EmailError.invalid_errors(batch) + end + end + + def batch_subject + self.email_batch.try(:subject) + end + end end diff --git a/web/Gemfile b/web/Gemfile index 27471d0a7..04415dab0 100644 --- a/web/Gemfile +++ b/web/Gemfile @@ -75,6 +75,7 @@ gem 'resque_mailer' gem 'quiet_assets', :group => :development gem 'bugsnag' gem 'multi_json', '1.9.0' +gem 'rest_client' group :development, :test do gem 'rspec-rails' @@ -131,4 +132,3 @@ group :package do gem 'fpm' end - diff --git a/websocket-gateway/Gemfile b/websocket-gateway/Gemfile index 5f8f740c6..7ffbb045f 100644 --- a/websocket-gateway/Gemfile +++ b/websocket-gateway/Gemfile @@ -42,9 +42,11 @@ gem 'resque' gem 'resque-retry' gem 'resque-failed-job-mailer' gem 'resque-lonely_job', '~> 1.0.0' +gem 'resque_mailer' gem 'geokit' gem 'geokit-rails', '2.0.1' gem 'mime-types', '1.25.1' +gem 'rest-client' group :development do gem 'pry'