feat: Add bulk imports API for contacts (#1724)

Gemfile

@@ -44,6 +44,8 @@ gem 'pg'
 gem 'redis'
 gem 'redis-namespace'
 gem 'redis-rack-cache'
+# super fast record imports in bulk
+gem 'activerecord-import'
 
 ##--- gems for server & infra configuration ---##
 gem 'dotenv-rails'

Gemfile.lock

@@ -62,6 +62,8 @@ GEM
     activerecord (6.0.3.4)
       activemodel (= 6.0.3.4)
       activesupport (= 6.0.3.4)
+    activerecord-import (1.0.7)
+      activerecord (>= 3.2)
     activestorage (6.0.3.4)
       actionpack (= 6.0.3.4)
       activejob (= 6.0.3.4)
@@ -582,6 +584,7 @@ PLATFORMS
 
 DEPENDENCIES
   action-cable-testing
+  activerecord-import
   acts-as-taggable-on
   administrate
   annotate

app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb

@@ -1,21 +1,14 @@
 class Api::V1::Accounts::Contacts::ContactInboxesController < Api::V1::Accounts::BaseController
   before_action :ensure_contact
   before_action :ensure_inbox, only: [:create]
-  before_action :validate_channel_type
 
   def create
     source_id = params[:source_id] || SecureRandom.uuid
-    @contact_inbox = ContactInbox.create(contact: @contact, inbox: @inbox, source_id: source_id)
+    @contact_inbox = ContactInbox.create!(contact: @contact, inbox: @inbox, source_id: source_id)
   end
 
   private
 
-  def validate_channel_type
-    return if @inbox.channel_type == 'Channel::Api'
-
-    render json: { error: 'Contact Inbox creation is only allowed in API inboxes' }, status: :unprocessable_entity
-  end
-
   def ensure_inbox
     @inbox = Current.account.inboxes.find(params[:inbox_id])
   end

app/controllers/api/v1/accounts/contacts_controller.rb

@@ -22,6 +22,14 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController
     @contacts = fetch_contact_last_seen_at(contacts)
   end
 
+  def import
+    ActiveRecord::Base.transaction do
+      import = Current.account.data_imports.create!(data_type: 'contacts')
+      import.import_file.attach(params[:import_file])
+    end
+    head :ok
+  end
+
   # returns online contacts
   def active
     contacts = Current.account.contacts.where(id: ::OnlineStatusTracker

app/jobs/data_import_job.rb

@@ -0,0 +1,46 @@
+# TODO: logic is written tailored to contact import since its the only import available
+# let's break this logic and clean this up in future
+
+class DataImportJob < ApplicationJob
+  queue_as :low
+
+  def perform(data_import)
+    contacts = []
+    data_import.update!(status: :processing)
+    csv = CSV.parse(data_import.import_file.download, headers: true)
+    csv.each { |row| contacts << build_contact(row.to_h.with_indifferent_access, data_import.account) }
+    result = Contact.import contacts, on_duplicate_key_update: :all, batch_size: 1000
+    data_import.update!(status: :completed, processed_records: csv.length - result.failed_instances.length, total_records: csv.length)
+  end
+
+  private
+
+  def build_contact(params, account)
+    # TODO: rather than doing the find or initialize individually lets fetch objects in bulk and update them in memory
+    contact = init_contact(params, account)
+
+    contact.name = params[:name] if params[:name].present?
+    contact.assign_attributes(custom_attributes: contact.custom_attributes.merge(params.except(:identifier, :email, :name)))
+
+    # since callbacks aren't triggered lets ensure a pubsub token
+    contact.pubsub_token ||= SecureRandom.base58(24)
+    contact
+  end
+
+  def get_identified_contacts(params, account)
+    identifier_contact = account.contacts.find_by(identifier: params[:identifier]) if params[:identifier]
+    email_contact = account.contacts.find_by(email: params[:email]) if params[:email]
+    [identifier_contact, email_contact]
+  end
+
+  def init_contact(params, account)
+    identifier_contact, email_contact = get_identified_contacts(params, account)
+
+    # intiating the new contact / contact attributes only by ensuring the identifier or email duplication errors won't occur
+    contact = identifier_contact
+    contact&.email = params[:email] if params[:email].present? && email_contact.blank?
+    contact ||= email_contact
+    contact ||= account.contacts.new(params.slice(:email, :identifier))
+    contact
+  end
+end

app/models/account.rb

@@ -34,6 +34,7 @@ class Account < ApplicationRecord
 
   has_many :account_users, dependent: :destroy
   has_many :agent_bot_inboxes, dependent: :destroy
+  has_many :data_imports, dependent: :destroy
   has_many :users, through: :account_users
   has_many :inboxes, dependent: :destroy
   has_many :conversations, dependent: :destroy

app/models/contact_inbox.rb

@@ -27,6 +27,7 @@ class ContactInbox < ApplicationRecord
   validates :inbox_id, presence: true
   validates :contact_id, presence: true
   validates :source_id, presence: true
+  validate :valid_source_id_format?
 
   belongs_to :contact
   belongs_to :inbox
@@ -47,4 +48,24 @@ class ContactInbox < ApplicationRecord
   def current_conversation
     conversations.last
   end
+
+  private
+
+  def validate_twilio_source_id
+    # https://www.twilio.com/docs/glossary/what-e164#regex-matching-for-e164
+    if inbox.channel.medium == 'sms' && !/^\+[1-9]\d{1,14}$/.match?(source_id)
+      errors.add(:source_id, 'invalid source id for twilio sms inbox. valid Regex /^\+[1-9]\d{1,14}$/')
+    elsif inbox.channel.medium == 'whatsapp' && !/^whatsapp:\+[1-9]\d{1,14}$/.match?(source_id)
+      errors.add(:source_id, 'invalid source id for twilio whatsapp inbox. valid Regex /^whatsapp:\+[1-9]\d{1,14}$/')
+    end
+  end
+
+  def validate_email_source_id
+    errors.add(:source_id, "invalid source id for Email inbox. valid Regex #{Device.email_regexp}") unless Devise.email_regexp.match?(source_id)
+  end
+
+  def valid_source_id_format?
+    validate_twilio_source_id if inbox.channel_type == 'Channel::TwilioSms'
+    validate_email_source_id if inbox.channel_type == 'Channel::Email'
+  end
 end

app/models/data_import.rb

@@ -0,0 +1,37 @@
+# == Schema Information
+#
+# Table name: data_imports
+#
+#  id                :bigint           not null, primary key
+#  data_type         :string           not null
+#  processed_records :integer
+#  processing_errors :text
+#  status            :integer          default("pending"), not null
+#  total_records     :integer
+#  created_at        :datetime         not null
+#  updated_at        :datetime         not null
+#  account_id        :bigint           not null
+#
+# Indexes
+#
+#  index_data_imports_on_account_id  (account_id)
+#
+# Foreign Keys
+#
+#  fk_rails_...  (account_id => accounts.id)
+#
+class DataImport < ApplicationRecord
+  belongs_to :account
+  validates :data_type, inclusion: { in: ['contacts'], message: '%<value>s is an invalid data type' }
+  enum status: { pending: 0, processing: 1, completed: 2, failed: 3 }
+
+  has_one_attached :import_file
+
+  after_create_commit :process_data_import
+
+  private
+
+  def process_data_import
+    DataImportJob.perform_later(self)
+  end
+end

app/policies/contact_policy.rb

@@ -7,6 +7,10 @@ class ContactPolicy < ApplicationPolicy
     true
   end
 
+  def import?
+    @account_user.administrator?
+  end
+
   def search?
     true
   end

config/routes.rb

@@ -67,6 +67,7 @@ Rails.application.routes.draw do
             collection do
               get :active
               get :search
+              post :import
             end
             scope module: :contacts do
               resources :conversations, only: [:index]

db/migrate/20210201150037_create_data_imports.rb

@@ -0,0 +1,14 @@
+class CreateDataImports < ActiveRecord::Migration[6.0]
+  def change
+    create_table :data_imports do |t|
+      t.references :account, null: false, foreign_key: true
+      t.string :data_type, null: false
+      t.integer :status, null: false, default: 0
+      t.text :processing_errors
+      t.integer :total_records
+      t.integer :processed_records
+
+      t.timestamps
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_01_26_121313) do
+ActiveRecord::Schema.define(version: 2021_02_01_150037) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_stat_statements"
@@ -241,6 +241,18 @@ ActiveRecord::Schema.define(version: 2021_01_26_121313) do
     t.index ["team_id"], name: "index_conversations_on_team_id"
   end
 
+  create_table "data_imports", force: :cascade do |t|
+    t.bigint "account_id", null: false
+    t.string "data_type", null: false
+    t.integer "status", default: 0, null: false
+    t.text "processing_errors"
+    t.integer "total_records"
+    t.integer "processed_records"
+    t.datetime "created_at", precision: 6, null: false
+    t.datetime "updated_at", precision: 6, null: false
+    t.index ["account_id"], name: "index_data_imports_on_account_id"
+  end
+
   create_table "email_templates", force: :cascade do |t|
     t.string "name", null: false
     t.text "body", null: false
@@ -585,6 +597,7 @@ ActiveRecord::Schema.define(version: 2021_01_26_121313) do
   add_foreign_key "contact_inboxes", "inboxes"
   add_foreign_key "conversations", "contact_inboxes"
   add_foreign_key "conversations", "teams"
+  add_foreign_key "data_imports", "accounts"
   add_foreign_key "team_members", "teams"
   add_foreign_key "team_members", "users"
   add_foreign_key "teams", "accounts"

spec/assets/contacts.csv

@@ -0,0 +1,26 @@
+id,first_name,last_name,email,gender,ip_address,identifier
+1,Clarice,Uzzell,cuzzell0@mozilla.org,Genderfluid,70.61.11.201,bb4e11cd-0f23-49da-a123-dcc1fec6852c
+2,Marieann,Creegan,mcreegan1@cornell.edu,Genderfluid,168.186.4.241,e60bab4c-9fbb-47eb-8f75-42025b789c47
+3,Nancey,Windibank,nwindibank2@bluehost.com,Agender,73.44.41.59,f793e813-4210-4bf3-a812-711418de25d2
+4,Sibel,Stennine,sstennine3@yellowbook.com,Genderqueer,115.249.27.155,d6e35a2d-d093-4437-a577-7df76316b937
+5,Tina,O'Lunney,tolunney4@si.edu,Bigender,219.181.212.8,3540d40a-5567-4f28-af98-5583a7ddbc56
+6,Quinn,Neve,qneve5@army.mil,Genderfluid,231.210.115.166,ba0e1bf0-c74b-41ce-8a2d-0b08fa0e5aa5
+7,Karylin,Gaunson,kgaunson6@tripod.com,Polygender,160.189.41.11,d24cac79-c81b-4b84-a33e-0441b7c6a981
+8,Jamison,Shenton,jshenton7@upenn.edu,Agender,53.94.18.201,29a7a8c0-c7f7-4af9-852f-761b1a784a7a
+9,Gavan,Threlfall,gthrelfall8@spotify.com,Genderfluid,18.87.247.249,847d4943-ddb5-47cc-8008-ed5092c675c5
+10,Katina,Hemmingway,khemmingway9@ameblo.jp,Non-binary,25.191.96.124,8f0b5efd-b6a8-4f1e-a1e3-b0ea8c9e3048
+11,Jillian,Deinhard,jdeinharda@canalblog.com,Female,11.211.174.93,bd952787-1b05-411f-9975-b916ec0950cc
+12,Blake,Finden,bfindenb@wsj.com,Female,47.26.205.153,12c95613-e49d-4fa2-86fb-deabb6ebe600
+13,Liane,Maxworthy,lmaxworthyc@un.org,Non-binary,157.196.34.166,36b68e4c-40d6-4e09-bf59-7db3b27b18f0
+14,Martynne,Ledley,mledleyd@sourceforge.net,Polygender,109.231.152.148,1856bceb-cb36-415c-8ffc-0527f3f750d8
+15,Katharina,Ruffli,krufflie@huffingtonpost.com,Genderfluid,20.43.146.179,604de5c9-b154-4279-8978-41fb71f0f773
+16,Tucker,Simmance,tsimmancef@bbc.co.uk,Bigender,179.76.226.171,0a8fc3a7-4986-4a51-a503-6c7f974c90ad
+17,Wenona,Martinson,wmartinsong@census.gov,Genderqueer,92.243.194.160,0e5ea6e3-6824-4e78-a6f5-672847eafa17
+18,Gretna,Vedyasov,gvedyasovh@lycos.com,Female,25.22.86.101,6becf55b-a7b5-48f6-8788-b89cae85b066
+19,Lurline,Abdon,labdoni@archive.org,Genderqueer,150.249.116.118,afa9429f-9034-4b06-9efa-980e01906ebf
+20,Fiann,Norcliff,fnorcliffj@istockphoto.com,Female,237.167.197.197,59f72dec-14ba-4d6e-b17c-0d962e69ffac
+21,Zed,Linn,zlinnk@phoca.cz,Genderqueer,88.102.64.113,95f7bc56-be92-4c9c-ad58-eff3e63c7bea
+22,Averyl,Simyson,asimysonl@livejournal.com,Agender,141.248.89.29,bde1fe59-c9bd-440c-bb39-79fe61dac1d1
+23,Camella,Blackadder,cblackadderm@nifty.com,Polygender,118.123.138.115,0c981752-5857-487c-b9b5-5d0253df740a
+24,Aurie,Spatig,aspatign@printfriendly.com,Polygender,157.45.102.235,4cf22bfb-2c3f-41d1-9993-6e3758e457ba
+25,Adrienne,Bellard,abellardo@cnn.com,Male,170.73.198.47,f10f9b8d-38ac-4e17-8a7d-d2e6a055f944

spec/controllers/api/v1/accounts/contacts/contact_inboxes_controller_spec.rb

@@ -3,7 +3,7 @@ require 'rails_helper'
 RSpec.describe '/api/v1/accounts/{account.id}/contacts/:id/contact_inboxes', type: :request do
   let(:account) { create(:account) }
   let(:contact) { create(:contact, account: account) }
-  let(:inbox_1) { create(:inbox, account: account) }
+  let(:channel_twilio_sms) { create(:channel_twilio_sms, account: account) }
   let(:channel_api) { create(:channel_api, account: account) }
   let(:user) { create(:user, account: account) }
 
@@ -28,10 +28,10 @@ RSpec.describe '/api/v1/accounts/{account.id}/contacts/:id/contact_inboxes', typ
         expect(contact.reload.contact_inboxes.map(&:inbox_id)).to include(channel_api.inbox.id)
       end
 
-      it 'throws error when its not an api inbox' do
+      it 'throws error for invalid source id' do
         expect do
           post "/api/v1/accounts/#{account.id}/contacts/#{contact.id}/contact_inboxes",
-               params: { inbox_id: inbox_1.id },
+               params: { inbox_id: channel_twilio_sms.inbox.id },
                headers: user.create_new_auth_token,
                as: :json
         end.to change(ContactInbox, :count).by(0)

spec/controllers/api/v1/accounts/contacts_controller_spec.rb

@@ -43,6 +43,43 @@ RSpec.describe 'Contacts API', type: :request do
     end
   end
 
+  describe 'POST /api/v1/accounts/{account.id}/contacts/import' do
+    context 'when it is an unauthenticated user' do
+      it 'returns unauthorized' do
+        post "/api/v1/accounts/#{account.id}/contacts/import"
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
+    context 'when it is an authenticated user with out permission' do
+      let(:agent) { create(:user, account: account, role: :agent) }
+
+      it 'returns unauthorized' do
+        post "/api/v1/accounts/#{account.id}/contacts/import",
+             headers: agent.create_new_auth_token,
+             as: :json
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
+    context 'when it is an authenticated user' do
+      let(:admin) { create(:user, account: account, role: :administrator) }
+
+      it 'creates a data import' do
+        file = fixture_file_upload(Rails.root.join('spec/assets/contacts.csv'), 'text/csv')
+        post "/api/v1/accounts/#{account.id}/contacts/import",
+             headers: admin.create_new_auth_token,
+             params: { import_file: file }
+
+        expect(response).to have_http_status(:success)
+        expect(account.data_imports.count).to eq(1)
+        expect(account.data_imports.first.import_file.attached?).to eq(true)
+      end
+    end
+  end
+
   describe 'GET /api/v1/accounts/{account.id}/contacts/active' do
     context 'when it is an unauthenticated user' do
       it 'returns unauthorized' do

spec/factories/channel/twilio_sms.rb

@@ -4,7 +4,9 @@ FactoryBot.define do
     account_sid { SecureRandom.uuid }
     sequence(:phone_number) { |n| "+123456789#{n}1" }
     medium { :sms }
-    inbox
     account
+    after(:build) do |channel|
+      channel.inbox ||= create(:inbox, account: channel.account)
+    end
   end
 end

spec/factories/contact_inbox.rb

@@ -4,6 +4,18 @@ FactoryBot.define do
   factory :contact_inbox do
     contact
     inbox
-    source_id { SecureRandom.uuid }
+
+    after(:build) { |contact_inbox| contact_inbox.source_id ||= generate_source_id(contact_inbox) }
+  end
+end
+
+def generate_source_id(contact_inbox)
+  case contact_inbox.inbox.channel_type
+  when 'Channel::TwilioSms'
+    contact_inbox.inbox.channel.medium == 'sms' ? Faker::PhoneNumber.cell_phone_in_e164 : "whatsapp:#{Faker::PhoneNumber.cell_phone_in_e164}"
+  when 'Channel::Email'
+    "#{SecureRandom.uuid}@acme.inc"
+  else
+    SecureRandom.uuid
   end
 end

spec/factories/data_import.rb

@@ -0,0 +1,7 @@
+FactoryBot.define do
+  factory :data_import do
+    data_type { 'contacts' }
+    import_file { Rack::Test::UploadedFile.new(Rails.root.join('spec/assets/contacts.csv'), 'text/csv') }
+    account
+  end
+end

spec/jobs/data_import_job_spec.rb

@@ -0,0 +1,24 @@
+require 'rails_helper'
+
+RSpec.describe DataImportJob, type: :job do
+  subject(:job) { described_class.perform_later(data_import) }
+
+  let!(:data_import) { create(:data_import) }
+
+  it 'queues the job' do
+    expect { job }.to have_enqueued_job(described_class)
+      .with(data_import)
+      .on_queue('low')
+  end
+
+  it 'imports data into the account' do
+    csv_length = CSV.parse(data_import.import_file.download, headers: true).length
+    described_class.perform_now(data_import)
+    expect(data_import.account.contacts.count).to eq(csv_length)
+    expect(data_import.reload.total_records).to eq(csv_length)
+    expect(data_import.reload.processed_records).to eq(csv_length)
+
+    # should generate pubsub tokens for contacts
+    expect(data_import.account.contacts.last.pubsub_token).present?
+  end
+end

spec/models/data_import_spec.rb

@@ -0,0 +1,13 @@
+require 'rails_helper'
+
+RSpec.describe DataImport, type: :model do
+  describe 'associations' do
+    it { is_expected.to belong_to(:account) }
+  end
+
+  describe 'validations' do
+    it 'returns false for invalid data type' do
+      expect(build(:data_import, data_type: 'Xyc').valid?).to eq false
+    end
+  end
+end