Feature: Contact Merge Action (#378)

.rubocop.yml

@@ -7,7 +7,7 @@ inherit_from: .rubocop_todo.yml
 Metrics/LineLength:
   Max: 150
 RSpec/ExampleLength:
-  Max: 10
+  Max: 15
 Documentation:
   Enabled: false
 Style/FrozenStringLiteralComment:

app/actions/contact_merge_action.rb

@@ -0,0 +1,36 @@
+class ContactMergeAction
+  pattr_initialize [:account!, :base_contact!, :mergee_contact!]
+
+  def perform
+    ActiveRecord::Base.transaction do
+      validate_contacts
+      merge_conversations
+      merge_contact_inboxes
+      remove_mergee_contact
+    end
+  end
+
+  private
+
+  def validate_contacts
+    return if belongs_to_account?(@base_contact) && belongs_to_account?(@mergee_contact)
+
+    raise Exception, 'contact does not belong to the account'
+  end
+
+  def belongs_to_account?(contact)
+    @account.id == contact.account_id
+  end
+
+  def merge_conversations
+    Conversation.where(contact_id: @mergee_contact.id).update(contact_id: @base_contact.id)
+  end
+
+  def merge_contact_inboxes
+    ContactInbox.where(contact_id: @mergee_contact.id).update(contact_id: @base_contact.id)
+  end
+
+  def remove_mergee_contact
+    @mergee_contact.destroy!
+  end
+end

app/controllers/api/v1/actions/contact_merges_controller.rb

@@ -0,0 +1,28 @@
+class Api::V1::Actions::ContactMergesController < Api::BaseController
+  before_action :set_base_contact, only: [:create]
+  before_action :set_mergee_contact, only: [:create]
+
+  def create
+    contact_merge_action = ContactMergeAction.new(
+      account: current_account,
+      base_contact: @base_contact,
+      mergee_contact: @mergee_contact
+    )
+    contact_merge_action.perform
+    render json: @base_contact
+  end
+
+  private
+
+  def set_base_contact
+    @base_contact = contacts.find(params[:base_contact_id])
+  end
+
+  def set_mergee_contact
+    @mergee_contact = contacts.find(params[:mergee_contact_id])
+  end
+
+  def contacts
+    @contacts ||= current_account.contacts
+  end
+end

config/routes.rb

@@ -29,6 +29,10 @@ Rails.application.routes.draw do
         resources :inboxes, only: [:create]
       end
 
+      namespace :actions do
+        resource :contact_merge, only: [:create]
+      end
+
       resource :profile, only: [:show, :update]
       resources :accounts, only: [:create]
       resources :inboxes, only: [:index, :destroy]

spec/actions/contact_merge_action_spec.rb

@@ -0,0 +1,47 @@
+require 'rails_helper'
+
+describe ::ContactMergeAction do
+  subject(:contact_merge) { described_class.new(account: account, base_contact: base_contact, mergee_contact: mergee_contact).perform }
+
+  let!(:account) { create(:account) }
+  let!(:base_contact) { create(:contact, account: account) }
+  let!(:mergee_contact) { create(:contact, account: account) }
+
+  before do
+    2.times.each { create(:conversation, contact: base_contact) }
+    2.times.each { create(:contact_inbox, contact: base_contact) }
+    2.times.each { create(:conversation, contact: mergee_contact) }
+    2.times.each { create(:contact_inbox, contact: mergee_contact) }
+  end
+
+  describe '#perform' do
+    it 'deletes mergee_contact' do
+      contact_merge
+      expect { mergee_contact.reload }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+
+    context 'when mergee contact has conversations' do
+      it 'moves the conversations to base contact' do
+        contact_merge
+        expect(base_contact.conversations.count).to be 4
+      end
+    end
+
+    context 'when mergee contact has contact inboxes' do
+      it 'moves the contact inboxes to base contact' do
+        contact_merge
+        expect(base_contact.contact_inboxes.count).to be 4
+      end
+    end
+
+    context 'when contacts belong to a different account' do
+      it 'throws an exception' do
+        new_account = create(:account)
+        expect do
+          described_class.new(account: new_account, base_contact: base_contact,
+                              mergee_contact: mergee_contact).perform
+        end .to raise_error('contact does not belong to the account')
+      end
+    end
+  end
+end

spec/controllers/api/v1/actions/contact_merges_controller_spec.rb

@@ -0,0 +1,41 @@
+require 'rails_helper'
+
+RSpec.describe 'Contact Merge Action API', type: :request do
+  let(:account) { create(:account) }
+  let!(:base_contact) { create(:contact, account: account) }
+  let!(:mergee_contact) { create(:contact, account: account) }
+
+  describe 'POST /api/v1/actions/contact_merge' do
+    context 'when it is an unauthenticated user' do
+      it 'returns unauthorized' do
+        post '/api/v1/actions/contact_merge'
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
+    context 'when it is an authenticated user' do
+      let(:agent) { create(:user, account: account, role: :agent) }
+      let(:merge_action) { double }
+
+      before do
+        allow(ContactMergeAction).to receive(:new).and_return(merge_action)
+        allow(merge_action).to receive(:perform)
+      end
+
+      it 'merges two contacts by calling contact merge action' do
+        post '/api/v1/actions/contact_merge',
+             params: { base_contact_id: base_contact.id, mergee_contact_id: mergee_contact.id },
+             headers: agent.create_new_auth_token,
+             as: :json
+
+        expect(response).to have_http_status(:success)
+        json_response = JSON.parse(response.body)
+        expect(json_response['id']).to eq(base_contact.id)
+        expected_params = { account: account, base_contact: base_contact, mergee_contact: mergee_contact }
+        expect(ContactMergeAction).to have_received(:new).with(expected_params)
+        expect(merge_action).to have_received(:perform)
+      end
+    end
+  end
+end

spec/controllers/api/v1/profiles_controller_spec.rb

@@ -4,7 +4,7 @@ RSpec.describe 'Profile API', type: :request do
   let(:account) { create(:account) }
 
   describe 'GET /api/v1/profile' do
-    context 'when unauthenticated user' do
+    context 'when it is an unauthenticated user' do
       it 'returns unauthorized' do
         get '/api/v1/profile'
 
@@ -12,7 +12,7 @@ RSpec.describe 'Profile API', type: :request do
       end
     end
 
-    context 'when it authenticated user' do
+    context 'when it is an authenticated user' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
       it 'returns current user information' do
@@ -29,7 +29,7 @@ RSpec.describe 'Profile API', type: :request do
   end
 
   describe 'PUT /api/v1/profile' do
-    context 'when unauthenticated user' do
+    context 'when it is an unauthenticated user' do
       it 'returns unauthorized' do
         put '/api/v1/profile'
 
@@ -37,12 +37,13 @@ RSpec.describe 'Profile API', type: :request do
       end
     end
 
-    context 'when it authenticated user' do
+    context 'when it is an authenticated user' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
       it 'updates the name & email' do
+        new_email = Faker::Internet.email
         put '/api/v1/profile',
-            params: { profile: { name: 'test', 'email': 'test@test.com' } },
+            params: { profile: { name: 'test', 'email': new_email } },
             headers: agent.create_new_auth_token,
             as: :json
 
@@ -51,7 +52,7 @@ RSpec.describe 'Profile API', type: :request do
         agent.reload
         expect(json_response['id']).to eq(agent.id)
         expect(json_response['email']).to eq(agent.email)
-        expect(agent.email).to eq('test@test.com')
+        expect(agent.email).to eq(new_email)
       end
 
       it 'updates the password' do
@@ -68,7 +69,7 @@ RSpec.describe 'Profile API', type: :request do
         expect(agent.avatar.attached?).to eq(false)
         file = fixture_file_upload(Rails.root.join('spec/assets/avatar.png'), 'image/png')
         put '/api/v1/profile',
-            params: { profile: { name: 'test', 'email': 'test@test.com', avatar: file } },
+            params: { profile: { avatar: file } },
             headers: agent.create_new_auth_token
 
         expect(response).to have_http_status(:success)

spec/factories/contact_inbox.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :contact_inbox do
+    contact
+    inbox
+    source_id { SecureRandom.uuid }
+  end
+end

spec/factories/conversations.rb

@@ -8,17 +8,14 @@ FactoryBot.define do
     agent_last_seen_at { Time.current }
     locked { false }
 
-    factory :complete_conversation do
+    after(:build) do |conversation|
-      after(:build) do |conversation|
+      conversation.account ||= create(:account)
-        conversation.account ||= create(:account)
+      conversation.inbox ||= create(
-        conversation.inbox ||= create(
+        :inbox,
-          :inbox,
+        account: conversation.account,
-          account: conversation.account,
+        channel: create(:channel_widget, account: conversation.account)
-          channel: create(:channel_widget, account: conversation.account)
+      )
-        )
+      conversation.contact ||= create(:contact, account: conversation.account)
-        conversation.contact ||= create(:contact, account: conversation.account)
-        conversation.assignee ||= create(:user)
-      end
     end
   end
 end

spec/finders/conversation_finder_spec.rb

@@ -11,10 +11,10 @@ describe ::ConversationFinder do
   before do
     create(:inbox_member, user: user_1, inbox: inbox)
     create(:inbox_member, user: user_2, inbox: inbox)
-    create(:complete_conversation, account: account, inbox: inbox, assignee: user_1)
+    create(:conversation, account: account, inbox: inbox, assignee: user_1)
-    create(:complete_conversation, account: account, inbox: inbox, assignee: user_1)
+    create(:conversation, account: account, inbox: inbox, assignee: user_1)
-    create(:complete_conversation, account: account, inbox: inbox, assignee: user_1, status: 'resolved')
+    create(:conversation, account: account, inbox: inbox, assignee: user_1, status: 'resolved')
-    create(:complete_conversation, account: account, inbox: inbox, assignee: user_2)
+    create(:conversation, account: account, inbox: inbox, assignee: user_2)
   end
 
   describe '#perform' do
@@ -40,7 +40,7 @@ describe ::ConversationFinder do
       let(:params) { { status: 'open', assignee_type_id: 0, page: 1 } }
 
       it 'returns paginated conversations' do
-        create_list(:complete_conversation, 50, account: account, inbox: inbox, assignee: user_1)
+        create_list(:conversation, 50, account: account, inbox: inbox, assignee: user_1)
         result = conversation_finder.perform
         expect(result[:conversations].count).to be 25
       end

spec/finders/message_finder_spec.rb

@@ -6,7 +6,7 @@ describe ::MessageFinder do
   let!(:account) { create(:account) }
   let!(:user) { create(:user, account: account) }
   let!(:inbox) { create(:inbox, account: account) }
-  let!(:conversation) { create(:complete_conversation, account: account, inbox: inbox, assignee: user) }
+  let!(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user) }
 
   before do
     create(:message, account: account, inbox: inbox, conversation: conversation)

spec/models/conversation_spec.rb

@@ -4,7 +4,7 @@ require 'rails_helper'
 
 RSpec.describe Conversation, type: :model do
   describe '.before_create' do
-    let(:conversation) { build(:complete_conversation, display_id: nil) }
+    let(:conversation) { build(:conversation, display_id: nil) }
 
     before do
       conversation.save
@@ -19,7 +19,7 @@ RSpec.describe Conversation, type: :model do
   describe '.after_update' do
     let(:account) { create(:account) }
     let(:conversation) do
-      create(:complete_conversation, status: 'open', account: account, assignee: old_assignee)
+      create(:conversation, status: 'open', account: account, assignee: old_assignee)
     end
     let(:old_assignee) do
       create(:user, email: 'agent1@example.com', account: account, role: :agent)
@@ -104,7 +104,7 @@ RSpec.describe Conversation, type: :model do
   describe '#update_assignee' do
     subject(:update_assignee) { conversation.update_assignee(agent) }
 
-    let(:conversation) { create(:complete_conversation, assignee: nil) }
+    let(:conversation) { create(:conversation, assignee: nil) }
     let(:agent) do
       create(:user, email: 'agent@example.com', account: conversation.account, role: :agent)
     end
@@ -118,7 +118,7 @@ RSpec.describe Conversation, type: :model do
   describe '#toggle_status' do
     subject(:toggle_status) { conversation.toggle_status }
 
-    let(:conversation) { create(:complete_conversation, status: :open) }
+    let(:conversation) { create(:conversation, status: :open) }
 
     it 'toggles conversation status' do
       expect(toggle_status).to eq(true)
@@ -129,7 +129,7 @@ RSpec.describe Conversation, type: :model do
   describe '#lock!' do
     subject(:lock!) { conversation.lock! }
 
-    let(:conversation) { create(:complete_conversation) }
+    let(:conversation) { create(:conversation) }
 
     it 'assigns locks the conversation' do
       expect(lock!).to eq(true)
@@ -140,7 +140,7 @@ RSpec.describe Conversation, type: :model do
   describe '#unlock!' do
     subject(:unlock!) { conversation.unlock! }
 
-    let(:conversation) { create(:complete_conversation) }
+    let(:conversation) { create(:conversation) }
 
     it 'unlocks the conversation' do
       expect(unlock!).to eq(true)
@@ -151,7 +151,7 @@ RSpec.describe Conversation, type: :model do
   describe 'unread_messages' do
     subject(:unread_messages) { conversation.unread_messages }
 
-    let(:conversation) { create(:complete_conversation, agent_last_seen_at: 1.hour.ago) }
+    let(:conversation) { create(:conversation, agent_last_seen_at: 1.hour.ago) }
     let(:message_params) do
       {
         conversation: conversation,
@@ -176,7 +176,7 @@ RSpec.describe Conversation, type: :model do
   describe 'unread_incoming_messages' do
     subject(:unread_incoming_messages) { conversation.unread_incoming_messages }
 
-    let(:conversation) { create(:complete_conversation, agent_last_seen_at: 1.hour.ago) }
+    let(:conversation) { create(:conversation, agent_last_seen_at: 1.hour.ago) }
     let(:message_params) do
       {
         conversation: conversation,
@@ -202,7 +202,7 @@ RSpec.describe Conversation, type: :model do
   describe '#push_event_data' do
     subject(:push_event_data) { conversation.push_event_data }
 
-    let(:conversation) { create(:complete_conversation) }
+    let(:conversation) { create(:conversation) }
     let(:expected_data) do
       {
         meta: {

spec/presenters/conversations/event_data_presenter_spec.rb

@@ -4,7 +4,7 @@ require 'rails_helper'
 
 RSpec.describe Conversations::EventDataPresenter do
   let(:presenter) { described_class.new(conversation) }
-  let(:conversation) { create(:complete_conversation) }
+  let(:conversation) { create(:conversation) }
 
   describe '#lock_data' do
     it { expect(presenter.lock_data).to eq(id: conversation.display_id, locked: false) }