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/controllers/api/sendgrid_controller.rb b/app/controllers/api/sendgrid_controller.rb index 7ed89d7..b7338c7 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_events_username, password: AppConstants.sendgrid_events_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..24d5dbb --- /dev/null +++ b/app/models/sendgrid_events.rb @@ -0,0 +1,172 @@ +require 'newrelic_rpm' + +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 + NewRelic::Agent.increment_metric("Custom/SendgridEvent/#{@name}", 1) + # 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 + super + # 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 + 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) + if member and email + UserActivityEvent.email_viewed!(member, email) + else + true + end + end + end + + 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) + if member and email + UserActivityEvent.email_clicked!(member, email) + else + true + end + end + end + + 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 + member.permanently_unsubscribe! + UserActivityEvent.email_spammed!(member, email) + else + true + end + end + end + + 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! + 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"] + email_id = evt["email_id"] + + 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 + 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 + end + +end diff --git a/config/constants.yml b/config/constants.yml index db034ad..5851191 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_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 e0a5004..361852e 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_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' + + 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)}, "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