From 7d85f2e046e716266bae6de975950a824018944a Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Tue, 24 Dec 2019 13:27:25 +0530 Subject: [PATCH] Feature: Contact Merge Action (#378) --- .rubocop.yml | 2 +- app/actions/contact_merge_action.rb | 36 ++++++++++++++ .../v1/actions/contact_merges_controller.rb | 28 +++++++++++ config/routes.rb | 4 ++ spec/actions/contact_merge_action_spec.rb | 47 +++++++++++++++++++ .../actions/contact_merges_controller_spec.rb | 41 ++++++++++++++++ .../api/v1/profiles_controller_spec.rb | 15 +++--- spec/factories/contact_inbox.rb | 9 ++++ spec/factories/conversations.rb | 19 ++++---- spec/finders/conversation_finder_spec.rb | 10 ++-- spec/finders/message_finder_spec.rb | 2 +- spec/models/conversation_spec.rb | 18 +++---- .../event_data_presenter_spec.rb | 2 +- 13 files changed, 198 insertions(+), 35 deletions(-) create mode 100644 app/actions/contact_merge_action.rb create mode 100644 app/controllers/api/v1/actions/contact_merges_controller.rb create mode 100644 spec/actions/contact_merge_action_spec.rb create mode 100644 spec/controllers/api/v1/actions/contact_merges_controller_spec.rb create mode 100644 spec/factories/contact_inbox.rb diff --git a/.rubocop.yml b/.rubocop.yml index f39c72801..248a11eed 100644 --- a/.rubocop.yml +++ b/.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: diff --git a/app/actions/contact_merge_action.rb b/app/actions/contact_merge_action.rb new file mode 100644 index 000000000..61a26e185 --- /dev/null +++ b/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 diff --git a/app/controllers/api/v1/actions/contact_merges_controller.rb b/app/controllers/api/v1/actions/contact_merges_controller.rb new file mode 100644 index 000000000..2eead4869 --- /dev/null +++ b/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 diff --git a/config/routes.rb b/config/routes.rb index e810700d9..74f36d59a 100644 --- a/config/routes.rb +++ b/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] diff --git a/spec/actions/contact_merge_action_spec.rb b/spec/actions/contact_merge_action_spec.rb new file mode 100644 index 000000000..7ffcaf1fa --- /dev/null +++ b/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 diff --git a/spec/controllers/api/v1/actions/contact_merges_controller_spec.rb b/spec/controllers/api/v1/actions/contact_merges_controller_spec.rb new file mode 100644 index 000000000..78b907025 --- /dev/null +++ b/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 diff --git a/spec/controllers/api/v1/profiles_controller_spec.rb b/spec/controllers/api/v1/profiles_controller_spec.rb index 23ac669b1..9355177ed 100644 --- a/spec/controllers/api/v1/profiles_controller_spec.rb +++ b/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) diff --git a/spec/factories/contact_inbox.rb b/spec/factories/contact_inbox.rb new file mode 100644 index 000000000..24b39cba4 --- /dev/null +++ b/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 diff --git a/spec/factories/conversations.rb b/spec/factories/conversations.rb index d1dee0df9..f89fc899c 100644 --- a/spec/factories/conversations.rb +++ b/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| - conversation.account ||= create(:account) - conversation.inbox ||= create( - :inbox, - account: conversation.account, - channel: create(:channel_widget, account: conversation.account) - ) - conversation.contact ||= create(:contact, account: conversation.account) - conversation.assignee ||= create(:user) - end + after(:build) do |conversation| + conversation.account ||= create(:account) + conversation.inbox ||= create( + :inbox, + account: conversation.account, + channel: create(:channel_widget, account: conversation.account) + ) + conversation.contact ||= create(:contact, account: conversation.account) end end end diff --git a/spec/finders/conversation_finder_spec.rb b/spec/finders/conversation_finder_spec.rb index e31d17267..24529efb8 100644 --- a/spec/finders/conversation_finder_spec.rb +++ b/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(:complete_conversation, account: account, inbox: inbox, assignee: user_1) - create(:complete_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_1) + create(:conversation, account: account, inbox: inbox, assignee: user_1) + create(:conversation, account: account, inbox: inbox, assignee: user_1, status: 'resolved') + 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 diff --git a/spec/finders/message_finder_spec.rb b/spec/finders/message_finder_spec.rb index 6676987b2..ca05a510d 100644 --- a/spec/finders/message_finder_spec.rb +++ b/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) diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 605611fcb..e5ddbeb96 100644 --- a/spec/models/conversation_spec.rb +++ b/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: { diff --git a/spec/presenters/conversations/event_data_presenter_spec.rb b/spec/presenters/conversations/event_data_presenter_spec.rb index 3f04519fb..d3d5210e7 100644 --- a/spec/presenters/conversations/event_data_presenter_spec.rb +++ b/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) }