From 48bbd5f9366b92bbce7a15460188b23cdd2d4f68 Mon Sep 17 00:00:00 2001 From: Noel Welsh Date: Fri, 28 Feb 2014 10:04:24 +0000 Subject: [PATCH 1/6] Process all Sendgrid Events Extend the Sendgrid event handler to process all events received from Sendgrid (using the Event Webhook v3 API). This includes email views, email clicks, unsubscribes, spam reports, and bounces. The endpoint uses basic auth to provide a bit of protection against attack. The username and password are set in the SENDGRID_USER and SENDGRID_PASSWORD environment variables respectively. They default to sendgrid (both username and password) if not set. When configuring Sendgrid these parameters must be set as described in the Sendgrid documentation. All event processing goes via delayed job to even out load. If you use Platform's event tracking this can lead to double counting. There isn't a general solution to this as multiple opens, views, etc. are valid. However, double counting isn't likely to make much difference in practice. --- app/controllers/api/sendgrid_controller.rb | 27 +-- app/models/sendgrid_events.rb | 167 +++++++++++++++ config/constants.yml | 2 + .../api/sendgrid_controller_spec.rb | 121 ++++++----- spec/models/sendgrid_events_spec.rb | 193 ++++++++++++++++++ 5 files changed, 447 insertions(+), 63 deletions(-) create mode 100644 app/models/sendgrid_events.rb create mode 100644 spec/models/sendgrid_events_spec.rb diff --git a/app/controllers/api/sendgrid_controller.rb b/app/controllers/api/sendgrid_controller.rb index 7ed89d7..2d70b9e 100644 --- a/app/controllers/api/sendgrid_controller.rb +++ b/app/controllers/api/sendgrid_controller.rb @@ -1,26 +1,21 @@ +require 'json' + class Api::SendgridController < Api::BaseController - def event_handler - member = User.find_by_movement_id_and_email(@movement.id, params[:email]) - head :ok and return if !member - member.permanently_unsubscribe! if should_unsubscribe? + http_basic_authenticate_with name: AppConstants.sendgrid_user, password: AppConstants.sendgrid_password - if spam? - email = Email.find(params[:email_id]) - UserActivityEvent.email_spammed!(member, email) + def event_handler + events = JSON.parse(request.body.read) + events.each do |evt| + handle_event(@movement.id, evt) end - head :ok - end - def should_unsubscribe? - hard_bounce? || spam? + head :ok end - def hard_bounce? - params[:event] == 'bounce' + def handle_event(movement_id, event) + evt = SendgridEvents::create(movement_id, event) + evt.delay.handle end - def spam? - params[:event] == 'spamreport' - end end diff --git a/app/models/sendgrid_events.rb b/app/models/sendgrid_events.rb new file mode 100644 index 0000000..9c2e306 --- /dev/null +++ b/app/models/sendgrid_events.rb @@ -0,0 +1,167 @@ +module SendgridEvents + + class Event + def initialize(movement_id, email_address, email_id) + @movement_id = movement_id + @email_address = email_address + @email_id = email_id + end + + # Returns truthy on success, false otherwise + # + # Note the success value only indicates that the handler could run + # without error. If you create a handler with a bogus email + # address, for instance, it will run succesfully but won't do + # anything. + def handle + # Do whatever this event needs to do + end + + def to_s + @name ||= "Event" + "#{@name}(#{@movement_id}, #{@email_address}, #{@email_id})" + end + end + + class Processed < Event + # Do nothing + @name = "Processed" + end + + class Dropped < Event + # This event is often raised when an email address is invalid but + # could also be raised if there is an error how SendGrid is + # called. Thus it isn't safe to unsubscribe a user generating this + # event. + # + # See: http://sendgrid.com/docs/API_Reference/Webhooks/event.html + # + # TODO: Remove the email_send event associated with this email + @name = "Dropped" + end + + class Delivered < Event + # We already record this when the email is sent to SendGrid so + # do nothing. + @name = "Delivered" + end + + class Deferred < Event + # We don't have a representation for this event so do nothing. + @name = "Deferred" + end + + class Bounce < Event + @name = "Bounce" + def handle + # Could not deliver, so unsubscribe this user. + member = User.find_by_movement_id_and_email(@movement_id, @email_address) + if member + member.permanently_unsubscribe! + else + true + end + end + end + + class Open < Event + @name = "Open" + def handle + # Register an email_viewed event + member = User.find_by_movement_id_and_email(@movement_id, @email_address) + email = Email.find_by_id(@email_id) + if member and email + UserActivityEvent.email_viewed!(member, email) + else + true + end + end + end + + class Click < Event + @name = "Click" + def handle + # Register an email_clicked event if we don't have one already + member = User.find_by_movement_id_and_email(@movement_id, @email_address) + email = Email.find_by_id(@email_id) + if member and email + UserActivityEvent.email_clicked!(member, email) + else + true + end + end + end + + class SpamReport < Event + @name = "SpamReport" + def handle + member = User.find_by_movement_id_and_email(@movement_id, @email_address) + email = Email.find_by_id(@email_id) + if member and email + member.permanently_unsubscribe! + UserActivityEvent.email_spammed!(member, email) + else + true + end + end + end + + class Unsubscribe < Event + @name = "Unsubscribe" + def handle + member = User.find_by_movement_id_and_email(@movement_id, @email_address) + if member + member.permanently_unsubscribe! + else + true + end + end + end + + + @@the_handlers = { + processed: Processed, + dropped: Dropped, + bounce: Bounce, + delivered: Delivered, + deferred: Deferred, + bounce: Bounce, + open: Open, + click: Click, + spamreport: SpamReport, + unsubscribe: Unsubscribe + } + + # Event that does nothing + @@the_noop = Event.new(0, 'dummy', 0) + + def self.noop + @@the_noop + end + + # Create an Event object from JSON + def self.create(movement_id, evt) + event = evt["event"] + email_address = evt["email"] + unique_args = evt["unique_args"] + email_id = if unique_args + unique_args["email_id"] + else + false + end + + if event and email_address and email_id + handler = @@the_handlers[event.to_sym] + if handler + handler.new(movement_id, email_address, email_id) + else + Rails.logger.warn "Could not find a handler to process SendGrid event #{evt}" + @@the_noop + end + else + Rails.logger.warn "Could not create a handler to process SendGrid event from #{evt}" + @@the_noop + end + end + +end diff --git a/config/constants.yml b/config/constants.yml index db034ad..1339d46 100644 --- a/config/constants.yml +++ b/config/constants.yml @@ -14,6 +14,8 @@ development: &default google_api_key: <%= ENV["GOOGLE_API_KEY"] %> google_maps_js_url: <%= ENV["GOOGLE_MAPS_JS_URL"] || "https://maps.google.com/maps/api/js?sensor=false" %> keyed_google_maps_js_url: <%= ENV["KEYED_GOOGLE_MAPS_JS_URL"] || "https://maps.googleapis.com/maps/api/js?key=#{ENV["GOOGLE_API_KEY"]}&sensor=false" %> + sendgrid_user: <%= ENV["SENDGRID_USER"] || "sendgrid" %> + sendgrid_password: <%= ENV["SENDGRID_PASSWORD"] || "sendgrid" %> production: <<: *default diff --git a/spec/controllers/api/sendgrid_controller_spec.rb b/spec/controllers/api/sendgrid_controller_spec.rb index e0a5004..9e7a5c9 100644 --- a/spec/controllers/api/sendgrid_controller_spec.rb +++ b/spec/controllers/api/sendgrid_controller_spec.rb @@ -1,63 +1,90 @@ require 'spec_helper' describe Api::SendgridController do - let(:walkfree) { FactoryGirl.create(:movement, :name => 'WalkFree') } - let(:allout) { FactoryGirl.create(:movement, :name => 'AllOut') } - let(:therules) { FactoryGirl.create(:movement, :name => 'therules') } - let(:walkfree_member) { FactoryGirl.create(:user, :email => 'member@movement.com',:movement => walkfree) } - let(:allout_member) { FactoryGirl.create(:user, :email => 'member@movement.com', :movement => allout) } - before do - User.stub(:find_by_movement_id_and_email).and_return(FactoryGirl.build(:user, :movement => walkfree)) - User.stub(:find_by_movement_id_and_email).with(walkfree.id, 'member@movement.com').and_return(walkfree_member) - User.stub(:find_by_movement_id_and_email).with(allout.id, 'member@movement.com').and_return(allout_member) - User.stub(:find_by_movement_id_and_email).with(therules.id, 'member-not-found@movement.com').and_return(nil) - end - - context '#event_handler' do - it 'should always respond success' do - post :event_handler, :movement_id => allout.id - response.code.should == '200' - end + before(:all) do + Delayed::Worker.delay_jobs = false + end - context 'with a bounce event' do - it 'should permanently unsubscribe the member from a specific movement' do - post :event_handler, :movement_id => allout.id, :email => 'member@movement.com', :event => 'bounce' - walkfree_member.should be_member - walkfree_member.can_subscribe?.should be_true + before(:each) do + @action_page = FactoryGirl.create(:action_page) + @movement = @action_page.movement + @unsubscribe = FactoryGirl.create(:unsubscribe_module, pages: [@action_page]) + @campaign = FactoryGirl.create(:campaign, movement: @movement) + @push = FactoryGirl.create(:push, campaign: @campaign) + @blast = FactoryGirl.create(:blast, push: @push) + @email = FactoryGirl.create(:email, blast: @blast) + @supporter = FactoryGirl.create(:user, + :email => "bob@example.com", + :movement => @movement, :is_member => true) + end - allout_member.should_not be_member - allout_member.can_subscribe?.should be_false - end - it 'should return 200 if member is nil' do - post :event_handler, :movement_id => therules.id, :email => 'member-not-found@movement.com', :event => 'bounce' - response.code.should == '200' - end + ## Helpers + + def handle_events(json, user: AppConstants.sendgrid_user, password: AppConstants.sendgrid_password) + @request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(user, password) + @request.env['RAW_POST_DATA'] = json + @request.env['HTTP_ACCEPT'] = 'application/json' + + post :event_handler, :movement_id => @movement.id + end + + def quote(str) + "\"#{str}\"" + end + + def find_by_email(email) + User.find_by_email_and_movement_id(email, @movement.id) + end + + def make_event(type, email_address, email_id) + %[{ "event": #{quote(type.to_s)}, "email": #{quote(email_address)}, "unique_args": {"email_id": #{quote(email_id.to_s)}} }] + end + + def make_events(events) + "[#{events.map { |evt| make_event(*evt) }.join(',')}]" + end + + def make_user(email) + FactoryGirl.create(:user, + :email => email, + :movement => @movement, :is_member => true) + end + + + ## Specs + + describe '#event_handler' do + it 'prevents unauthorized access' do + handle_events("{}", password: "ff334444g") + expect(response.code).to eq("401") end - context 'with a spamreport event' do - let(:spammed_email) { FactoryGirl.create(:email) } - before { Email.stub(:find).with(spammed_email.id.to_s).and_return(spammed_email) } + it 'always responds with success to authorized requests' do + handle_events("{}") + expect(response.code).to eq("200") + end - it 'should permanently unsubscribe the member from a specific movement' do - UserActivityEvent.stub(:email_spammed!).with(allout_member, spammed_email) - post :event_handler, :movement_id => allout.id, :email => 'member@movement.com', :event => 'spamreport', :email_id => spammed_email.id - walkfree_member.should be_member - walkfree_member.can_subscribe?.should be_true + context 'with a list of events' do + let(:supporter1) { make_user('one@example.com') } + let(:supporter2) { make_user('two@example.com') } - allout_member.should_not be_member - allout_member.can_subscribe?.should be_false - end + it 'processes all events' do + expect(find_by_email(supporter1.email).is_member).to be_true + expect(find_by_email(supporter2.email).is_member).to be_true - it 'should report an email spammed event' do - UserActivityEvent.should_receive(:email_spammed!).with(allout_member, spammed_email) - post :event_handler, :movement_id => allout.id, :email => 'member@movement.com', :event => 'spamreport', :email_id => spammed_email.id - end + events = make_events([ + [:bounce, supporter1.email, @email.id], + [:spamreport, supporter2.email, @email.id] + ]) + handle_events(events) - it 'should return 200 if member is nil' do - post :event_handler, :movement_id => therules.id, :email => 'member-not-found@movement.com', :event => 'spamreport' - response.code.should == '200' + expect(response.code).to eq("200") + expect(find_by_email(supporter1.email).is_member).to be_false + expect(find_by_email(supporter2.email).is_member).to be_false end end + + # Correct handling of individual events is tested in the spec for SendgridEvents end end diff --git a/spec/models/sendgrid_events_spec.rb b/spec/models/sendgrid_events_spec.rb new file mode 100644 index 0000000..fa55203 --- /dev/null +++ b/spec/models/sendgrid_events_spec.rb @@ -0,0 +1,193 @@ +require "spec_helper" + +describe "SendgridEvents" do + + before(:each) do + @action_page = FactoryGirl.create(:action_page) + @movement = @action_page.movement + @unsubscribe = FactoryGirl.create(:unsubscribe_module, pages: [@action_page]) + @campaign = FactoryGirl.create(:campaign, movement: @movement) + @push = FactoryGirl.create(:push, campaign: @campaign) + @blast = FactoryGirl.create(:blast, push: @push) + @email = FactoryGirl.create(:email, blast: @blast) + @supporter = FactoryGirl.create(:user, + :email => "bob@example.com", + :movement => @movement, :is_member => true) + end + + + ### Helper functions + + def find_by_email(email) + User.find_by_email_and_movement_id(email, @movement.id) + end + + def event(type, email_address, email_id) + klass = + case type + when :processed + SendgridEvents::Processed + when :dropped + SendgridEvents::Dropped + when :bounce + SendgridEvents::Bounce + when :delivered + SendgridEvents::Delivered + when :deferred + SendgridEvents::Deferred + when :bounce + SendgridEvents::Bounce + when :open + SendgridEvents::Open + when :click + SendgridEvents::Click + when :spamreport + SendgridEvents::SpamReport + when :unsubscribe + SendgridEvents::Unsubscribe + end + klass.new(@movement.id, email_address, email_id) + end + + def event_and_handle(type, email_address: @supporter.email, email_id: @email.id) + evt = event(type, email_address, email_id) + result = evt.handle + expect(result).to be_true + result + end + + def event_with_bad_email_address(type) + bad_email = "dawg@example.com" + expect(find_by_email(bad_email)).to be_nil + event_and_handle(type, email_address: bad_email) + end + + def event_with_bad_email_id(type) + bad_id = 123456 + expect(Email.where(id: bad_id).count).to eq(0) + event_and_handle(type, email_id: bad_id) + end + + + ### Specs + + describe "noop" do + it "does nothing" do + noop = SendgridEvents::noop + noop.handle + end + end + + + describe "::SpamReport" do + it "unsubscribes the supporter" do + expect(find_by_email(@supporter.email).is_member).to be_true + + event_and_handle(:spamreport) + + expect(find_by_email(@supporter.email).is_member).to be_false + expect(UserActivityEvent.unsubscriptions.where(:user_id => @supporter.id).first).to be_true + end + + it "records spam activity" do + expect(find_by_email(@supporter.email).is_member).to be_true + + event_and_handle(:spamreport) + + expect(PushSpammedEmail.where(email_id: @email.id, user_id: @supporter.id).first).to be_true + end + + it "does not fail if the supporter doesn't exist" do + event_with_bad_email_address(:spamreport) + expect(PushSpammedEmail.count).to eq(0) + end + + it "does not fail if the email doesn't exist" do + event_with_bad_email_id(:spamreport) + expect(PushSpammedEmail.count).to eq(0) + end + end + + + describe "::Unsubscribe" do + it "unsubscribes the supporter" do + expect(find_by_email(@supporter.email).is_member).to be_true + + event_and_handle(:unsubscribe) + + expect(find_by_email(@supporter.email).is_member).to be_false + expect(UserActivityEvent.unsubscriptions.where(:user_id => @supporter.id).first).to be_true + end + + it "does not fail if the supporter doesn't exist" do + event_with_bad_email_address(:unsubscribe) + expect(UserActivityEvent.unsubscriptions.where(:user_id => @supporter.id).count).to eq(0) + end + + it "does not fail if the email doesn't exist" do + event_with_bad_email_id(:unsubscribe) + expect(UserActivityEvent.unsubscriptions.where(:user_id => @supporter.id).count).to eq(1) + end + end + + + describe "::Click" do + it "records a click" do + event_and_handle(:click) + + expect(PushClickedEmail.where(email_id: @email.id, user_id: @supporter.id).count).to eq(1) + end + + it "does not fail if the supporter doesn't exist" do + event_with_bad_email_address(:click) + expect(PushClickedEmail.count).to eq(0) + end + + it "does not fail if the email doesn't exist" do + event_with_bad_email_id(:click) + expect(PushClickedEmail.count).to eq(0) + end + end + + + describe "::Open" do + it "records a view" do + event_and_handle(:open) + + expect(PushViewedEmail.where(email_id: @email.id, user_id: @supporter.id).count).to eq(1) + end + + it "does not fail if the supporter doesn't exist" do + event_with_bad_email_address(:open) + expect(PushViewedEmail.count).to eq(0) + end + + it "does not fail if the email doesn't exist" do + event_with_bad_email_id(:open) + expect(PushViewedEmail.count).to eq(0) + end + end + + + describe "::Bounce" do + it "unsubscribes the supporter" do + expect(find_by_email(@supporter.email).is_member).to be_true + + event_and_handle(:bounce) + + expect(find_by_email(@supporter.email).is_member).to be_false + expect(UserActivityEvent.unsubscriptions.where(:user_id => @supporter.id).first).to be_true + end + + it "does not fail if the supporter doesn't exist" do + event_with_bad_email_address(:bounce) + expect(UserActivityEvent.unsubscriptions.where(:user_id => @supporter.id).count).to eq(0) + end + + it "does not fail if the email doesn't exist" do + event_with_bad_email_id(:open) + expect(UserActivityEvent.unsubscriptions.where(user_id: @supporter.id).count).to eq(0) + end + end + +end From 6003cca4e5e3d093900c7bdc9d1a5b45f31c50c8 Mon Sep 17 00:00:00 2001 From: Noel Welsh Date: Mon, 3 Mar 2014 10:11:06 +0000 Subject: [PATCH 2/6] Change SENDGRID_USER to SENDGRID_USERNAME for consistency with existing code --- app/controllers/api/sendgrid_controller.rb | 2 +- config/constants.yml | 2 +- spec/controllers/api/sendgrid_controller_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/sendgrid_controller.rb b/app/controllers/api/sendgrid_controller.rb index 2d70b9e..5d86496 100644 --- a/app/controllers/api/sendgrid_controller.rb +++ b/app/controllers/api/sendgrid_controller.rb @@ -2,7 +2,7 @@ class Api::SendgridController < Api::BaseController - http_basic_authenticate_with name: AppConstants.sendgrid_user, password: AppConstants.sendgrid_password + http_basic_authenticate_with name: AppConstants.sendgrid_username, password: AppConstants.sendgrid_password def event_handler events = JSON.parse(request.body.read) diff --git a/config/constants.yml b/config/constants.yml index 1339d46..1020a52 100644 --- a/config/constants.yml +++ b/config/constants.yml @@ -14,7 +14,7 @@ development: &default google_api_key: <%= ENV["GOOGLE_API_KEY"] %> google_maps_js_url: <%= ENV["GOOGLE_MAPS_JS_URL"] || "https://maps.google.com/maps/api/js?sensor=false" %> keyed_google_maps_js_url: <%= ENV["KEYED_GOOGLE_MAPS_JS_URL"] || "https://maps.googleapis.com/maps/api/js?key=#{ENV["GOOGLE_API_KEY"]}&sensor=false" %> - sendgrid_user: <%= ENV["SENDGRID_USER"] || "sendgrid" %> + sendgrid_username: <%= ENV["SENDGRID_USERNAME"] || "sendgrid" %> sendgrid_password: <%= ENV["SENDGRID_PASSWORD"] || "sendgrid" %> production: diff --git a/spec/controllers/api/sendgrid_controller_spec.rb b/spec/controllers/api/sendgrid_controller_spec.rb index 9e7a5c9..6ebce8b 100644 --- a/spec/controllers/api/sendgrid_controller_spec.rb +++ b/spec/controllers/api/sendgrid_controller_spec.rb @@ -21,7 +21,7 @@ ## Helpers - def handle_events(json, user: AppConstants.sendgrid_user, password: AppConstants.sendgrid_password) + def handle_events(json, user: AppConstants.sendgrid_username, password: AppConstants.sendgrid_password) @request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(user, password) @request.env['RAW_POST_DATA'] = json @request.env['HTTP_ACCEPT'] = 'application/json' From 5631ecd1a2c5622ec37e1640bf46e7fceb520c97 Mon Sep 17 00:00:00 2001 From: Noel Welsh Date: Mon, 3 Mar 2014 14:51:07 +0000 Subject: [PATCH 3/6] Distinguish between SendGrid API credentials and event handler credentials The SendGrid event handler was reusing the SendGrid API credentials. I decided this was a bad idea and it should have its own credentials, as they will probably be less secure. --- app/controllers/api/sendgrid_controller.rb | 2 +- config/constants.yml | 4 ++-- spec/controllers/api/sendgrid_controller_spec.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/sendgrid_controller.rb b/app/controllers/api/sendgrid_controller.rb index 5d86496..b7338c7 100644 --- a/app/controllers/api/sendgrid_controller.rb +++ b/app/controllers/api/sendgrid_controller.rb @@ -2,7 +2,7 @@ class Api::SendgridController < Api::BaseController - http_basic_authenticate_with name: AppConstants.sendgrid_username, password: AppConstants.sendgrid_password + http_basic_authenticate_with name: AppConstants.sendgrid_events_username, password: AppConstants.sendgrid_events_password def event_handler events = JSON.parse(request.body.read) diff --git a/config/constants.yml b/config/constants.yml index 1020a52..5851191 100644 --- a/config/constants.yml +++ b/config/constants.yml @@ -14,8 +14,8 @@ development: &default google_api_key: <%= ENV["GOOGLE_API_KEY"] %> google_maps_js_url: <%= ENV["GOOGLE_MAPS_JS_URL"] || "https://maps.google.com/maps/api/js?sensor=false" %> keyed_google_maps_js_url: <%= ENV["KEYED_GOOGLE_MAPS_JS_URL"] || "https://maps.googleapis.com/maps/api/js?key=#{ENV["GOOGLE_API_KEY"]}&sensor=false" %> - sendgrid_username: <%= ENV["SENDGRID_USERNAME"] || "sendgrid" %> - sendgrid_password: <%= ENV["SENDGRID_PASSWORD"] || "sendgrid" %> + sendgrid_events_username: <%= ENV["SENDGRID_EVENTS_USERNAME"] || "sendgrid" %> + sendgrid_events_password: <%= ENV["SENDGRID_EVENTS_PASSWORD"] || "sendgrid" %> production: <<: *default diff --git a/spec/controllers/api/sendgrid_controller_spec.rb b/spec/controllers/api/sendgrid_controller_spec.rb index 6ebce8b..38ae87d 100644 --- a/spec/controllers/api/sendgrid_controller_spec.rb +++ b/spec/controllers/api/sendgrid_controller_spec.rb @@ -21,7 +21,7 @@ ## Helpers - def handle_events(json, user: AppConstants.sendgrid_username, password: AppConstants.sendgrid_password) + def handle_events(json, user: AppConstants.sendgrid_events_username, password: AppConstants.sendgrid_events_password) @request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(user, password) @request.env['RAW_POST_DATA'] = json @request.env['HTTP_ACCEPT'] = 'application/json' From 68c35eb96c25328533207e26af4faae9eaee13ce Mon Sep 17 00:00:00 2001 From: Noel Welsh Date: Tue, 11 Mar 2014 10:39:53 +0000 Subject: [PATCH 4/6] Track events with New Relic custom metrics For greater insight into what is happening with event processing, track events and errors with New Relic custom metric under the Custom/SendgridEvent path. --- Gemfile | 2 +- Gemfile.lock | 4 ++-- app/models/sendgrid_events.rb | 10 ++++++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index c98fd96..94dc0ad 100644 --- a/Gemfile +++ b/Gemfile @@ -35,7 +35,7 @@ gem "pdfkit" gem 'enumerated_attribute' gem 'seed-fu' gem 'activerecord-import' -gem 'newrelic_rpm' +gem 'newrelic_rpm', '~> 3.7' gem 'redcarpet' gem 'activemodel-warnings' gem 'airbrake' diff --git a/Gemfile.lock b/Gemfile.lock index d9b6473..120391a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -246,7 +246,7 @@ GEM multi_json (1.7.9) mysql2 (0.3.11) netrc (0.7.7) - newrelic_rpm (3.5.6.55) + newrelic_rpm (3.7.3.199) nokogiri (1.6.0) mini_portile (~> 0.5.0) oink (0.10.1) @@ -480,7 +480,7 @@ DEPENDENCIES memcachier money mysql2 (= 0.3.11) - newrelic_rpm + newrelic_rpm (~> 3.7) nokogiri oink paperclip diff --git a/app/models/sendgrid_events.rb b/app/models/sendgrid_events.rb index 9c2e306..afb3e5e 100644 --- a/app/models/sendgrid_events.rb +++ b/app/models/sendgrid_events.rb @@ -1,3 +1,5 @@ +require 'newrelic_rpm' + module SendgridEvents class Event @@ -14,6 +16,7 @@ def initialize(movement_id, email_address, email_id) # address, for instance, it will run succesfully but won't do # anything. def handle + NewRelic::Agent.increment_metric("Custom/SendgridEvent/@name", 1) # Do whatever this event needs to do end @@ -54,6 +57,7 @@ class Deferred < Event class Bounce < Event @name = "Bounce" def handle + super # Could not deliver, so unsubscribe this user. member = User.find_by_movement_id_and_email(@movement_id, @email_address) if member @@ -67,6 +71,7 @@ def handle class Open < Event @name = "Open" def handle + super # Register an email_viewed event member = User.find_by_movement_id_and_email(@movement_id, @email_address) email = Email.find_by_id(@email_id) @@ -81,6 +86,7 @@ def handle class Click < Event @name = "Click" def handle + super # Register an email_clicked event if we don't have one already member = User.find_by_movement_id_and_email(@movement_id, @email_address) email = Email.find_by_id(@email_id) @@ -95,6 +101,7 @@ def handle class SpamReport < Event @name = "SpamReport" def handle + super member = User.find_by_movement_id_and_email(@movement_id, @email_address) email = Email.find_by_id(@email_id) if member and email @@ -109,6 +116,7 @@ def handle class Unsubscribe < Event @name = "Unsubscribe" def handle + super member = User.find_by_movement_id_and_email(@movement_id, @email_address) if member member.permanently_unsubscribe! @@ -155,10 +163,12 @@ def self.create(movement_id, evt) if handler handler.new(movement_id, email_address, email_id) else + NewRelic::Agent.increment_metric('Custom/SendgridEvent/NoHandler', 1) Rails.logger.warn "Could not find a handler to process SendGrid event #{evt}" @@the_noop end else + NewRelic::Agent.increment_metric('Custom/SendgridEvent/BadData', 1) Rails.logger.warn "Could not create a handler to process SendGrid event from #{evt}" @@the_noop end From 9789161db4a97bf528174cdb46b6d369de9a6439 Mon Sep 17 00:00:00 2001 From: Noel Welsh Date: Tue, 11 Mar 2014 16:02:20 +0000 Subject: [PATCH 5/6] Correct event format SendGrid has flattened their JSON format for events. Match that change. --- app/models/sendgrid_events.rb | 7 +------ spec/controllers/api/sendgrid_controller_spec.rb | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/app/models/sendgrid_events.rb b/app/models/sendgrid_events.rb index afb3e5e..4b22793 100644 --- a/app/models/sendgrid_events.rb +++ b/app/models/sendgrid_events.rb @@ -151,12 +151,7 @@ def self.noop def self.create(movement_id, evt) event = evt["event"] email_address = evt["email"] - unique_args = evt["unique_args"] - email_id = if unique_args - unique_args["email_id"] - else - false - end + email_id = evt["email_id"] if event and email_address and email_id handler = @@the_handlers[event.to_sym] diff --git a/spec/controllers/api/sendgrid_controller_spec.rb b/spec/controllers/api/sendgrid_controller_spec.rb index 38ae87d..361852e 100644 --- a/spec/controllers/api/sendgrid_controller_spec.rb +++ b/spec/controllers/api/sendgrid_controller_spec.rb @@ -38,7 +38,7 @@ def find_by_email(email) end def make_event(type, email_address, email_id) - %[{ "event": #{quote(type.to_s)}, "email": #{quote(email_address)}, "unique_args": {"email_id": #{quote(email_id.to_s)}} }] + %[{ "event": #{quote(type.to_s)}, "email": #{quote(email_address)}, "email_id": #{quote(email_id.to_s)} }] end def make_events(events) From 61b585af2a6be2ca5929e218009351af14a86ea9 Mon Sep 17 00:00:00 2001 From: Noel Welsh Date: Fri, 28 Mar 2014 09:36:21 +0000 Subject: [PATCH 6/6] Fix typo in string interpolation --- app/models/sendgrid_events.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/sendgrid_events.rb b/app/models/sendgrid_events.rb index 4b22793..24d5dbb 100644 --- a/app/models/sendgrid_events.rb +++ b/app/models/sendgrid_events.rb @@ -16,7 +16,7 @@ def initialize(movement_id, email_address, email_id) # address, for instance, it will run succesfully but won't do # anything. def handle - NewRelic::Agent.increment_metric("Custom/SendgridEvent/@name", 1) + NewRelic::Agent.increment_metric("Custom/SendgridEvent/#{@name}", 1) # Do whatever this event needs to do end