Mailchimp integration

Gemfile

@@ -31,6 +31,9 @@ gem 'sdoc', '~> 0.4.0', group: :doc
 # Use Capistrano for deployment
 # gem 'capistrano-rails', group: :development
 
+gem 'sidekiq'
+gem 'gibbon'
+
 group :development, :test do
   # Call 'byebug' anywhere in the code to stop execution and get a debugger console
   gem 'byebug'
@@ -45,3 +48,6 @@ group :development do
   gem 'spring'
 end
 
+group :test do
+  gem 'webmock'
+end

Gemfile.lock

@@ -35,6 +35,8 @@ GEM
       minitest (~> 5.1)
       thread_safe (~> 0.3, >= 0.3.4)
       tzinfo (~> 1.1)
+    addressable (2.5.2)
+      public_suffix (>= 2.0.2, < 4.0)
     angular_rails_csrf (1.0.4)
       rails (>= 3, < 5)
     angularjs-rails (1.4.8)
@@ -51,14 +53,23 @@ GEM
       execjs
     coffee-script-source (1.10.0)
     concurrent-ruby (1.0.5)
+    connection_pool (2.2.2)
+    crack (0.4.3)
+      safe_yaml (~> 1.0.0)
     crass (1.0.4)
     debug_inspector (0.0.2)
     diff-lcs (1.3)
     erubis (2.7.0)
     execjs (2.6.0)
+    faraday (0.15.2)
+      multipart-post (>= 1.2, < 3)
     ffi (1.9.23)
+    gibbon (3.2.0)
+      faraday (>= 0.9.1)
+      multi_json (>= 1.11.0)
     globalid (0.4.1)
       activesupport (>= 4.2.0)
+    hashdiff (0.3.7)
     i18n (0.9.5)
       concurrent-ruby (~> 1.0)
     jbuilder (2.4.0)
@@ -74,9 +85,13 @@ GEM
     mini_portile2 (2.3.0)
     minitest (5.11.3)
     multi_json (1.11.2)
+    multipart-post (2.0.0)
     nokogiri (1.8.2)
       mini_portile2 (~> 2.3.0)
+    public_suffix (3.0.2)
     rack (1.6.9)
+    rack-protection (2.0.3)
+      rack
     rack-test (0.6.3)
       rack (>= 1.0)
     rails (4.2.10)
@@ -109,6 +124,7 @@ GEM
       ffi (>= 0.5.0, < 2)
     rdoc (4.2.1)
       json (~> 1.4)
+    redis (4.0.1)
     rspec-core (3.7.1)
       rspec-support (~> 3.7.0)
     rspec-expectations (3.7.0)
@@ -126,6 +142,7 @@ GEM
       rspec-mocks (~> 3.7.0)
       rspec-support (~> 3.7.0)
     rspec-support (3.7.1)
+    safe_yaml (1.0.4)
     sass (3.5.5)
       sass-listen (~> 4.0.0)
     sass-listen (4.0.0)
@@ -140,6 +157,11 @@ GEM
     sdoc (0.4.1)
       json (~> 1.7, >= 1.7.7)
       rdoc (~> 4.0)
+    sidekiq (5.1.3)
+      concurrent-ruby (~> 1.0)
+      connection_pool (~> 2.2, >= 2.2.0)
+      rack-protection (>= 1.5.0)
+      redis (>= 3.3.5, < 5)
     spring (1.6.2)
     sprockets (3.7.1)
       concurrent-ruby (~> 1.0)
@@ -162,6 +184,10 @@ GEM
       binding_of_caller (>= 0.7.2)
       railties (>= 4.0)
       sprockets-rails (>= 2.0, < 4.0)
+    webmock (3.4.2)
+      addressable (>= 2.3.6)
+      crack (>= 0.3.2)
+      hashdiff
 
 PLATFORMS
   ruby
@@ -171,15 +197,18 @@ DEPENDENCIES
   angularjs-rails
   byebug
   coffee-rails (~> 4.1.0)
+  gibbon
   jbuilder (~> 2.0)
   rails (= 4.2.10)
   rspec-rails
   sass-rails (~> 5.0)
   sdoc (~> 0.4.0)
+  sidekiq
   spring
   sqlite3
   uglifier (>= 1.3.0)
   web-console (~> 2.0)
+  webmock
 
 BUNDLED WITH
-   1.16.1
+   1.16.2

app/controllers/entries_controller.rb

@@ -5,6 +5,7 @@ def create
     @entry = Entry.new(entry_params)
 
     if @entry.save
+      MailingListSubscriberJob.perform_later @entry.id
       render json: {success: true}
     else
       render json: {success: false, errors: @entry.errors}
@@ -14,6 +15,6 @@ def create
   private
     # Never trust parameters from the scary internet, only allow the white list through.
     def entry_params
-      params.require(:entry).permit(:competition_id, :name, :email)
+      params.require(:entry).permit(:competition_id, :given_name, :family_name, :email)
     end
 end

app/jobs/mailing_list_subscriber_job.rb

@@ -0,0 +1,12 @@
+class MailingListSubscriberJob < ActiveJob::Base
+  queue_as :default
+
+  def perform(entry_id)
+    entry = Entry.find(entry_id)
+
+    MailingList
+      .find(entry.competition.mailing_list_id)
+      .add_subscriber(given_name: entry.given_name, family_name: entry.family_name,
+        email: entry.email)
+  end
+end

app/models/entry.rb

@@ -8,7 +8,7 @@ class Entry < ActiveRecord::Base
   validates_presence_of :competition, inverse_of: :entries
   validates_presence_of :email
   validates_format_of :email, :with => EMAIL_REGEX, allow_blank: true, allow_nil: true
-  validates_presence_of :name, if: :requires_name
+  validates_presence_of :given_name, :family_name, if: :requires_name
   validates_uniqueness_of :email, scope: :competition, message: "has already entered this competition"
 
   private

app/views/competitions/entrant_page.erb

@@ -6,7 +6,7 @@
 
     <form name="entry">
       <div id="error_explanation" ng-if="ctrl.errors">
-        Sorry, ther was a problem saving your entry:
+        Sorry, there was a problem saving your entry:
         <ul>
           <li ng-repeat="(field, errors) in ctrl.errors">
             {{field}}:
@@ -16,8 +16,13 @@
       </div>
 
       <p ng-if="ctrl.competition.requires_entry_name">
-        <label for="name">Name</label>
-        <input id="name" name="name" ng-model="ctrl.entry.name">
+        <label for="given_name">Given Name</label>
+        <input id="given_name" name="given_name" ng-model="ctrl.entry.given_name">
+      </p>
+
+      <p ng-if="ctrl.competition.requires_entry_name">
+        <label for="family_name">Family Name</label>
+        <input id="family_name" name="family_name" ng-model="ctrl.entry.family_name">
       </p>
 
       <p>

config/application.rb

@@ -11,6 +11,7 @@ class Application < Rails::Application
     # Settings in config/environments/* take precedence over those specified here.
     # Application configuration should go into files in config/initializers
     # -- all .rb files in that directory are automatically loaded.
+    config.autoload_paths += %W(#{config.root}/lib)
 
     # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone.
     # Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC.
@@ -22,5 +23,7 @@ class Application < Rails::Application
 
     # Do not swallow errors in after_commit/after_rollback callbacks.
     config.active_record.raise_in_transactional_callbacks = true
+
+    config.active_job.queue_adapter = :sidekiq
   end
 end

config/initializers/gibbon.rb

@@ -0,0 +1 @@
+Gibbon::Request.api_key = Rails.application.secrets.mailchimp_api_key

config/routes.rb

@@ -1,5 +1,4 @@
 Rails.application.routes.draw do
-  resources :entries
   # The priority is based upon order of creation: first created -> highest priority.
   # See how all your routes lay out with "rake routes".
 

config/secrets.yml

@@ -12,11 +12,14 @@
 
 development:
   secret_key_base: 6d8541683d523f8b9ad80440983b1a0e3845533bf03e9caa0df5e477295b95d279695c86f0338a1fbb29ec9e7470dab7b7a1bc4c8b5167dbd449efa904787212
+  mailchimp_api_key: <%= ENV["MAILCHIMP_API_KEY"] %>
 
 test:
   secret_key_base: c77d08b24c34c9f962d0cf8dfe4a61843d2fa543acfc7c9d1bb478a9048d7c3b09491151f0a5b7ace298864344191c5be1351d7eefdbdf3a4c77918a64e8e2b4
+  mailchimp_api_key: secretapikeyhere-us18
 
 # Do not keep production secrets in the repository,
 # instead read values from the environment.
 production:
   secret_key_base: <%= ENV["SECRET_KEY_BASE"] %>
+  mailchimp_api_key: <%= ENV["MAILCHIMP_API_KEY"] %>

db/migrate/20180615064619_add_mailing_list_id_to_competitions.rb

@@ -0,0 +1,10 @@
+class AddMailingListIdToCompetitions < ActiveRecord::Migration
+  def change
+    add_column :competitions, :mailing_list_id, :string
+
+    Competition.reset_column_information
+
+    Competition.find(1).update!(mailing_list_id: "a94641097a")
+    Competition.find(2).update!(mailing_list_id: "4827de065a")
+  end
+end

db/migrate/20180617055027_split_entry_name_into_given_family_names.rb

@@ -0,0 +1,32 @@
+class SplitEntryNameIntoGivenFamilyNames < ActiveRecord::Migration
+  def up
+    add_column :entries, :given_name, :string
+    add_column :entries, :family_name, :string
+
+    Entry.reset_column_information
+
+    Entry.where.not(name: nil).find_each do |e|
+      puts e.name
+      name_parts = e.name.split(" ", 2)
+      e.given_name = name_parts[0]
+      e.family_name = name_parts[1]
+      e.save!
+    end
+
+    remove_column :entries, :name
+  end
+
+  def down
+    add_column :entries, :name, :string
+
+    Entry.reset_column_information
+
+    Entry.where.not(given_name: nil).find_each do |e|
+      e.name = "#{e.given_name} #{e.family_name}"
+      e.save!
+    end
+
+    remove_column :entries, :given_name
+    remove_column :entries, :family_name
+  end
+end

db/schema.rb

@@ -11,21 +11,23 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160125020403) do
+ActiveRecord::Schema.define(version: 20180617055027) do
 
   create_table "competitions", force: :cascade do |t|
     t.string   "name"
     t.boolean  "requires_entry_name", default: true
     t.datetime "created_at",                         null: false
     t.datetime "updated_at",                         null: false
+    t.string   "mailing_list_id"
   end
 
   create_table "entries", force: :cascade do |t|
     t.integer  "competition_id"
-    t.text     "name"
     t.text     "email"
     t.datetime "created_at",     null: false
     t.datetime "updated_at",     null: false
+    t.string   "given_name"
+    t.string   "family_name"
   end
 
   add_index "entries", ["competition_id", "email"], name: "index_entries_on_competition_id_and_email", unique: true

doc/considerations.md

@@ -0,0 +1,17 @@
+# Thought Processes and Other Considerations
+
+* To minimize performance impact for users, accessing the external API should be done outside the main controller process, ie. in a background job. I've added a basic Sidekiq setup for this purpose.
+
+* I've decided that Mailchimp integration with competition management is out of the scope of this task. In the real app, when managing a competition, there could be several possibilities:
+  * The admin could be given the option of selecting an existing Mailchimp mailing list to subscribe users to, or
+  * A new mailing list is auto-created for each new competition.
+Either way, the ID of that mailing list will be stored with the Competition record. I've created some test mailing lists in my personal Mailchimp account and hardcoded those via a data migration.
+
+* Secrets support was minimal in Rails 4.2, so I've relied on loading my Mailchimp API key from an ENV var in both dev and production. If using Rails 5 I'd use encrypted secrets, or a library like Figaro.
+
+* For i18n purposes, I decided against splitting the data entered into the name field, into first and last name components - there's no way to do this totally correctly even in English. Instead, I elected to change the one "Name" field into two - for given name and family name.
+  * I've used naive string splitting for existing data only because I know there isn't any existing data that would fail by doing this :)
+  * Alternatively, Mailchimp could have been configured to store a single "Name" field, but then this would prevent things like addressing emails with just a user's given name.
+
+* Possible optimizations include:
+  * Batching subscriptions into groups of a certain size or time range, to save hitting the API too many times

lib/mailing_list.rb

@@ -0,0 +1,36 @@
+require "digest"
+
+class MailingList
+  class APIError < ::StandardError; end
+
+  def self.find(id)
+    new(id)
+  end
+
+  def add_subscriber(details)
+    email = details[:email]
+
+    raw_request.members(email_hash(email)).upsert(body: {email_address: email, status: "subscribed",
+      merge_fields: { FNAME: details[:given_name], LNAME: details[:family_name]}})
+    true
+  rescue Gibbon::MailChimpError => e
+    raise APIError.new e.body
+  end
+
+  private
+
+  def initialize(id)
+    @id = id
+  end
+
+  # The request object should be reinstantiated for each API call.
+  # Each request keeps a list of "path parts" and that list resets after each call - but we always
+  # want to keep the list details in that path.
+  def raw_request
+    Gibbon::Request.lists(@id)
+  end
+
+  def email_hash(email)
+    Digest::MD5.hexdigest email
+  end
+end

spec/controllers/entries_controller_spec.rb

@@ -0,0 +1,28 @@
+require "rails_helper"
+
+RSpec.describe EntriesController do
+  let(:competition) { Competition.create!(name: "Stargate Competition") }
+
+  describe "#create" do
+    context "when the entry is saved successfully" do
+
+      it "adds the entry to the competition mailing list" do
+        allow(MailingListSubscriberJob).to receive(:perform_later)
+
+        post :create, entry: { competition_id: competition, given_name: "Daniel",
+          family_name: "Jackson", email: "djackson@example.com" }
+
+        entry = Entry.find_by!(email: "djackson@example.com")
+        expect(MailingListSubscriberJob).to have_received(:perform_later).with(entry.id)
+      end
+    end
+
+    context "when the entry could not be saved" do
+      it "does not attempt to add the entry to the competition mailing list" do
+        expect(MailingListSubscriberJob).not_to receive(:perform_later)
+
+        post :create, entry: { competition_id: competition }
+      end
+    end
+  end
+end

spec/jobs/mailing_list_subscriber_job_spec.rb

@@ -0,0 +1,18 @@
+require 'rails_helper'
+
+RSpec.describe MailingListSubscriberJob, type: :job do
+  describe "#perform" do
+    it "subscribes the user to the correct mailing list for the competition" do
+      competition = Competition.create!(name: "Stargate competition", mailing_list_id: "12345")
+      entry = competition.entries.create!(given_name: "Samantha", family_name: "Carter",
+        email: "scarter@example.com")
+
+      mailing_list_double = instance_double(MailingList)
+      expect(MailingList).to receive(:find).with("12345") { mailing_list_double }
+      expect(mailing_list_double).to receive(:add_subscriber).with(given_name: "Samantha",
+        family_name: "Carter", email: "scarter@example.com")
+
+      MailingListSubscriberJob.perform_now(entry.id)
+    end
+  end
+end