diff --git a/.rubocop.yml b/.rubocop.yml index dafd9a620..3665ad2e3 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -16,6 +16,7 @@ Metrics/ClassLength: - 'app/models/message.rb' - 'app/builders/messages/facebook/message_builder.rb' - 'app/controllers/api/v1/accounts/contacts_controller.rb' + - 'app/controllers/api/v1/accounts/conversations_controller.rb' - 'app/listeners/action_cable_listener.rb' - 'app/models/conversation.rb' RSpec/ExampleLength: diff --git a/app/builders/contact_inbox_builder.rb b/app/builders/contact_inbox_builder.rb index 1b8782c51..8fcd2b158 100644 --- a/app/builders/contact_inbox_builder.rb +++ b/app/builders/contact_inbox_builder.rb @@ -1,13 +1,12 @@ +# This Builder will create a contact inbox with specified attributes. If the contact inbox already exists, it will be returned. +# For Specific Channels like whatsapp, email etc . it smartly generated appropriate the source id when none is provided. + class ContactInboxBuilder - pattr_initialize [:contact_id!, :inbox_id!, :source_id] + pattr_initialize [:contact, :inbox, :source_id, { hmac_verified: false }] def perform - @contact = Contact.find(contact_id) - @inbox = @contact.account.inboxes.find(inbox_id) - return unless ['Channel::TwilioSms', 'Channel::Sms', 'Channel::Email', 'Channel::Api', 'Channel::Whatsapp'].include? @inbox.channel_type - - source_id = @source_id || generate_source_id - create_contact_inbox(source_id) if source_id.present? + @source_id ||= generate_source_id + create_contact_inbox if source_id.present? end private @@ -19,23 +18,37 @@ class ContactInboxBuilder when 'Channel::Whatsapp' wa_source_id when 'Channel::Email' - @contact.email + email_source_id when 'Channel::Sms' - @contact.phone_number - when 'Channel::Api' + phone_source_id + when 'Channel::Api', 'Channel::WebWidget' SecureRandom.uuid + else + raise "Unsupported operation for this channel: #{@inbox.channel_type}" end end + def email_source_id + raise ActionController::ParameterMissing, 'contact email' unless @contact.email + + @contact.email + end + + def phone_source_id + raise ActionController::ParameterMissing, 'contact phone number' unless @contact.phone_number + + @contact.phone_number + end + def wa_source_id - return unless @contact.phone_number + raise ActionController::ParameterMissing, 'contact phone number' unless @contact.phone_number # whatsapp doesn't want the + in e164 format @contact.phone_number.delete('+').to_s end def twilio_source_id - return unless @contact.phone_number + raise ActionController::ParameterMissing, 'contact phone number' unless @contact.phone_number case @inbox.channel.medium when 'sms' @@ -45,11 +58,11 @@ class ContactInboxBuilder end end - def create_contact_inbox(source_id) - ::ContactInbox.find_or_create_by!( + def create_contact_inbox + ::ContactInbox.create_with(hmac_verified: hmac_verified || false).find_or_create_by!( contact_id: @contact.id, inbox_id: @inbox.id, - source_id: source_id + source_id: @source_id ) end end diff --git a/app/builders/contact_builder.rb b/app/builders/contact_inbox_with_contact_builder.rb similarity index 51% rename from app/builders/contact_builder.rb rename to app/builders/contact_inbox_with_contact_builder.rb index 938072643..d97f64cfe 100644 --- a/app/builders/contact_builder.rb +++ b/app/builders/contact_inbox_with_contact_builder.rb @@ -1,25 +1,47 @@ -class ContactBuilder - pattr_initialize [:source_id!, :inbox!, :contact_attributes!, :hmac_verified] +# This Builder will create a contact and contact inbox with specified attributes. +# If an existing identified contact exisits, it will be returned. +# for contact inbox logic it uses the contact inbox builder + +class ContactInboxWithContactBuilder + pattr_initialize [:inbox!, :contact_attributes!, :source_id, :hmac_verified] def perform - contact_inbox = inbox.contact_inboxes.find_by(source_id: source_id) - return contact_inbox if contact_inbox + find_or_create_contact_and_contact_inbox + # in case of race conditions where contact is created by another thread + # we will try to find the contact and create a contact inbox + rescue ActiveRecord::RecordNotUnique + find_or_create_contact_and_contact_inbox + end - build_contact_inbox + def find_or_create_contact_and_contact_inbox + @contact_inbox = inbox.contact_inboxes.find_by(source_id: source_id) if source_id.present? + return @contact_inbox if @contact_inbox + + ActiveRecord::Base.transaction(requires_new: true) do + build_contact_with_contact_inbox + update_contact_avatar(@contact) unless @contact.avatar.attached? + @contact_inbox + end end private + def build_contact_with_contact_inbox + @contact = find_contact || create_contact + @contact_inbox = create_contact_inbox + end + def account @account ||= inbox.account end - def create_contact_inbox(contact) - ::ContactInbox.create_with(hmac_verified: hmac_verified || false).find_or_create_by!( - contact_id: contact.id, - inbox_id: inbox.id, - source_id: source_id - ) + def create_contact_inbox + ContactInboxBuilder.new( + contact: @contact, + inbox: @inbox, + source_id: @source_id, + hmac_verified: hmac_verified + ).perform end def update_contact_avatar(contact) @@ -61,16 +83,4 @@ class ContactBuilder account.contacts.find_by(phone_number: phone_number) end - - def build_contact_inbox - ActiveRecord::Base.transaction do - contact = find_contact || create_contact - contact_inbox = create_contact_inbox(contact) - update_contact_avatar(contact) - contact_inbox - rescue StandardError => e - Rails.logger.error e - raise e - end - end end diff --git a/app/builders/messages/facebook/message_builder.rb b/app/builders/messages/facebook/message_builder.rb index 9f670602a..42d567e54 100644 --- a/app/builders/messages/facebook/message_builder.rb +++ b/app/builders/messages/facebook/message_builder.rb @@ -22,10 +22,9 @@ class Messages::Facebook::MessageBuilder < Messages::Messenger::MessageBuilder return if @inbox.channel.reauthorization_required? ActiveRecord::Base.transaction do - build_contact + build_contact_inbox build_message end - ensure_contact_avatar rescue Koala::Facebook::AuthenticationError @inbox.channel.authorization_error! rescue StandardError => e @@ -35,15 +34,12 @@ class Messages::Facebook::MessageBuilder < Messages::Messenger::MessageBuilder private - def contact - @contact ||= @inbox.contact_inboxes.find_by(source_id: @sender_id)&.contact - end - - def build_contact - return if contact.present? - - @contact = Contact.create!(contact_params.except(:remote_avatar_url)) - @contact_inbox = ContactInbox.find_or_create_by!(contact: contact, inbox: @inbox, source_id: @sender_id) + def build_contact_inbox + @contact_inbox = ::ContactInboxWithContactBuilder.new( + source_id: @sender_id, + inbox: @inbox, + contact_attributes: contact_params + ).perform end def build_message @@ -54,19 +50,11 @@ class Messages::Facebook::MessageBuilder < Messages::Messenger::MessageBuilder end end - def ensure_contact_avatar - return if contact_params[:remote_avatar_url].blank? - return if @contact.avatar.attached? - - Avatar::AvatarFromUrlJob.perform_later(@contact, contact_params[:remote_avatar_url]) - end - def conversation @conversation ||= Conversation.find_by(conversation_params) || build_conversation end def build_conversation - @contact_inbox ||= contact.contact_inboxes.find_by!(source_id: @sender_id) Conversation.create!(conversation_params.merge( contact_inbox_id: @contact_inbox.id )) @@ -94,7 +82,7 @@ class Messages::Facebook::MessageBuilder < Messages::Messenger::MessageBuilder { account_id: @inbox.account_id, inbox_id: @inbox.id, - contact_id: contact.id + contact_id: @contact_inbox.contact_id } end @@ -105,7 +93,7 @@ class Messages::Facebook::MessageBuilder < Messages::Messenger::MessageBuilder message_type: @message_type, content: response.content, source_id: response.identifier, - sender: @outgoing_echo ? nil : contact + sender: @outgoing_echo ? nil : @contact_inbox.contact } end @@ -113,7 +101,7 @@ class Messages::Facebook::MessageBuilder < Messages::Messenger::MessageBuilder { name: "#{result['first_name'] || 'John'} #{result['last_name'] || 'Doe'}", account_id: @inbox.account_id, - remote_avatar_url: result['profile_pic'] || '' + avatar_url: result['profile_pic'] } end diff --git a/app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb b/app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb index fdcdcaf9e..b4287ae08 100644 --- a/app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb +++ b/app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb @@ -2,8 +2,11 @@ class Api::V1::Accounts::Contacts::ContactInboxesController < Api::V1::Accounts: before_action :ensure_inbox, only: [:create] def create - source_id = params[:source_id] || SecureRandom.uuid - @contact_inbox = ContactInbox.create!(contact: @contact, inbox: @inbox, source_id: source_id) + @contact_inbox = ContactInboxBuilder.new( + contact: @contact, + inbox: @inbox, + source_id: params[:source_id] + ).perform end private diff --git a/app/controllers/api/v1/accounts/contacts_controller.rb b/app/controllers/api/v1/accounts/contacts_controller.rb index 1c56e9c04..b86b973df 100644 --- a/app/controllers/api/v1/accounts/contacts_controller.rb +++ b/app/controllers/api/v1/accounts/contacts_controller.rb @@ -134,8 +134,11 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController return if params[:inbox_id].blank? inbox = Current.account.inboxes.find(params[:inbox_id]) - source_id = params[:source_id] || SecureRandom.uuid - ContactInbox.create!(contact: @contact, inbox: inbox, source_id: source_id) + ContactInboxBuilder.new( + contact: @contact, + inbox: inbox, + source_id: params[:source_id] + ).perform end def permitted_params diff --git a/app/controllers/api/v1/accounts/conversations_controller.rb b/app/controllers/api/v1/accounts/conversations_controller.rb index 0515eabca..8734a3dd4 100644 --- a/app/controllers/api/v1/accounts/conversations_controller.rb +++ b/app/controllers/api/v1/accounts/conversations_controller.rb @@ -3,7 +3,7 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro include DateRangeHelper before_action :conversation, except: [:index, :meta, :search, :create, :filter] - before_action :contact_inbox, only: [:create] + before_action :inbox, :contact, :contact_inbox, only: [:create] def index result = conversation_finder.perform @@ -109,22 +109,35 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro authorize @conversation.inbox, :show? end + def inbox + return if params[:inbox_id].blank? + + @inbox = Current.account.inboxes.find(params[:inbox_id]) + authorize @inbox, :show? + end + + def contact + return if params[:contact_id].blank? + + @contact = Current.account.contacts.find(params[:contact_id]) + end + def contact_inbox @contact_inbox = build_contact_inbox + # fallback for the old case where we do look up only using source id + # In future we need to change this and make sure we do look up on combination of inbox_id and source_id + # and deprecate the support of passing only source_id as the param @contact_inbox ||= ::ContactInbox.find_by!(source_id: params[:source_id]) authorize @contact_inbox.inbox, :show? end def build_contact_inbox - return if params[:contact_id].blank? || params[:inbox_id].blank? - - inbox = Current.account.inboxes.find(params[:inbox_id]) - authorize inbox, :show? + return if @inbox.blank? || @contact.blank? ContactInboxBuilder.new( - contact_id: params[:contact_id], - inbox_id: inbox.id, + contact: @contact, + inbox: @inbox, source_id: params[:source_id] ).perform end diff --git a/app/controllers/concerns/request_exception_handler.rb b/app/controllers/concerns/request_exception_handler.rb index 6e9ed04cc..2f53fdc2b 100644 --- a/app/controllers/concerns/request_exception_handler.rb +++ b/app/controllers/concerns/request_exception_handler.rb @@ -13,6 +13,8 @@ module RequestExceptionHandler render_not_found_error('Resource could not be found') rescue Pundit::NotAuthorizedError render_unauthorized('You are not authorized to do this action') + rescue ActionController::ParameterMissing => e + render_could_not_create_error(e.message) ensure # to address the thread variable leak issues in Puma/Thin webserver Current.reset diff --git a/app/controllers/public/api/v1/inboxes/contacts_controller.rb b/app/controllers/public/api/v1/inboxes/contacts_controller.rb index eb794f2a0..1fde3051e 100644 --- a/app/controllers/public/api/v1/inboxes/contacts_controller.rb +++ b/app/controllers/public/api/v1/inboxes/contacts_controller.rb @@ -4,7 +4,7 @@ class Public::Api::V1::Inboxes::ContactsController < Public::Api::V1::InboxesCon def create source_id = params[:source_id] || SecureRandom.uuid - @contact_inbox = ::ContactBuilder.new( + @contact_inbox = ::ContactInboxWithContactBuilder.new( source_id: source_id, inbox: @inbox_channel.inbox, contact_attributes: permitted_params.except(:identifier, :identifier_hash) diff --git a/app/mailboxes/mailbox_helper.rb b/app/mailboxes/mailbox_helper.rb index 1519343ca..216d5c2c3 100644 --- a/app/mailboxes/mailbox_helper.rb +++ b/app/mailboxes/mailbox_helper.rb @@ -34,7 +34,7 @@ module MailboxHelper end def create_contact - @contact_inbox = ::ContactBuilder.new( + @contact_inbox = ::ContactInboxWithContactBuilder.new( source_id: processed_mail.original_sender, inbox: @inbox, contact_attributes: { diff --git a/app/models/channel/facebook_page.rb b/app/models/channel/facebook_page.rb index 2153708e2..bba45f326 100644 --- a/app/models/channel/facebook_page.rb +++ b/app/models/channel/facebook_page.rb @@ -37,16 +37,11 @@ class Channel::FacebookPage < ApplicationRecord end def create_contact_inbox(instagram_id, name) - ActiveRecord::Base.transaction do - contact = inbox.account.contacts.create!(name: name) - ::ContactInbox.create!( - contact_id: contact.id, - inbox_id: inbox.id, - source_id: instagram_id - ) - rescue StandardError => e - Rails.logger.error e - end + @contact_inbox = ::ContactInboxWithContactBuilder.new({ + source_id: instagram_id, + inbox: inbox, + contact_attributes: { name: name } + }).perform end def subscribe diff --git a/app/models/channel/twitter_profile.rb b/app/models/channel/twitter_profile.rb index 4f6fa7ba1..d0f765e9f 100644 --- a/app/models/channel/twitter_profile.rb +++ b/app/models/channel/twitter_profile.rb @@ -32,16 +32,11 @@ class Channel::TwitterProfile < ApplicationRecord end def create_contact_inbox(profile_id, name, additional_attributes) - ActiveRecord::Base.transaction do - contact = inbox.account.contacts.create!(additional_attributes: additional_attributes, name: name) - ::ContactInbox.create!( - contact_id: contact.id, - inbox_id: inbox.id, - source_id: profile_id - ) - rescue StandardError => e - Rails.logger.error e - end + ::ContactInboxWithContactBuilder.new({ + source_id: profile_id, + inbox: inbox, + contact_attributes: { name: name, additional_attributes: additional_attributes } + }).perform end def twitter_client diff --git a/app/models/channel/web_widget.rb b/app/models/channel/web_widget.rb index b85443633..59d392892 100644 --- a/app/models/channel/web_widget.rb +++ b/app/models/channel/web_widget.rb @@ -98,19 +98,9 @@ class Channel::WebWidget < ApplicationRecord end def create_contact_inbox(additional_attributes = {}) - ActiveRecord::Base.transaction do - contact = inbox.account.contacts.create!( - name: ::Haikunator.haikunate(1000), - additional_attributes: additional_attributes - ) - contact_inbox = ::ContactInbox.create!( - contact_id: contact.id, - inbox_id: inbox.id, - source_id: SecureRandom.uuid - ) - contact_inbox - rescue StandardError => e - Rails.logger.error e - end + ::ContactInboxWithContactBuilder.new({ + inbox: inbox, + contact_attributes: { additional_attributes: additional_attributes } + }).perform end end diff --git a/app/services/line/incoming_message_service.rb b/app/services/line/incoming_message_service.rb index 535c03dc7..48a52eb8f 100644 --- a/app/services/line/incoming_message_service.rb +++ b/app/services/line/incoming_message_service.rb @@ -81,7 +81,7 @@ class Line::IncomingMessageService end def set_contact - contact_inbox = ::ContactBuilder.new( + contact_inbox = ::ContactInboxWithContactBuilder.new( source_id: line_contact_info['userId'], inbox: inbox, contact_attributes: contact_attributes diff --git a/app/services/sms/incoming_message_service.rb b/app/services/sms/incoming_message_service.rb index 7ee6e3e63..7aa22b19e 100644 --- a/app/services/sms/incoming_message_service.rb +++ b/app/services/sms/incoming_message_service.rb @@ -37,7 +37,7 @@ class Sms::IncomingMessageService end def set_contact - contact_inbox = ::ContactBuilder.new( + contact_inbox = ::ContactInboxWithContactBuilder.new( source_id: params[:from], inbox: @inbox, contact_attributes: contact_attributes diff --git a/app/services/telegram/incoming_message_service.rb b/app/services/telegram/incoming_message_service.rb index a26bda14e..e8ab8a72c 100644 --- a/app/services/telegram/incoming_message_service.rb +++ b/app/services/telegram/incoming_message_service.rb @@ -31,7 +31,7 @@ class Telegram::IncomingMessageService end def set_contact - contact_inbox = ::ContactBuilder.new( + contact_inbox = ::ContactInboxWithContactBuilder.new( source_id: params[:message][:from][:id], inbox: inbox, contact_attributes: contact_attributes diff --git a/app/services/twilio/incoming_message_service.rb b/app/services/twilio/incoming_message_service.rb index 50c77111c..4473131df 100644 --- a/app/services/twilio/incoming_message_service.rb +++ b/app/services/twilio/incoming_message_service.rb @@ -47,7 +47,7 @@ class Twilio::IncomingMessageService end def set_contact - contact_inbox = ::ContactBuilder.new( + contact_inbox = ::ContactInboxWithContactBuilder.new( source_id: params[:From], inbox: inbox, contact_attributes: contact_attributes diff --git a/app/services/whatsapp/incoming_message_base_service.rb b/app/services/whatsapp/incoming_message_base_service.rb index 40dafaced..da08e02d6 100644 --- a/app/services/whatsapp/incoming_message_base_service.rb +++ b/app/services/whatsapp/incoming_message_base_service.rb @@ -48,7 +48,7 @@ class Whatsapp::IncomingMessageBaseService contact_params = @processed_params[:contacts]&.first return if contact_params.blank? - contact_inbox = ::ContactBuilder.new( + contact_inbox = ::ContactInboxWithContactBuilder.new( source_id: contact_params[:wa_id], inbox: inbox, contact_attributes: { name: contact_params.dig(:profile, :name), phone_number: "+#{@processed_params[:messages].first[:from]}" } diff --git a/db/seeds.rb b/db/seeds.rb index c4c7eda71..ead862669 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -46,8 +46,13 @@ unless Rails.env.production? inbox = Inbox.create!(channel: web_widget, account: account, name: 'Acme Support') InboxMember.create!(user: user, inbox: inbox) - contact = Contact.create!(name: 'jane', email: 'jane@example.com', phone_number: '+2320000', account: account) - contact_inbox = ContactInbox.create!(inbox: inbox, contact: contact, source_id: user.id, hmac_verified: true) + contact = ::ContactInboxWithContactBuilder.new( + source_id: user.id, + inbox: inbox, + hmac_verified: true, + contact_attributes: { name: 'jane', email: 'jane@example.com', phone_number: '+2320000' } + ).perform&.contact + conversation = Conversation.create!( account: account, inbox: inbox, diff --git a/spec/builders/contact_inbox_builder_spec.rb b/spec/builders/contact_inbox_builder_spec.rb index 47210f6ca..46b0d749c 100644 --- a/spec/builders/contact_inbox_builder_spec.rb +++ b/spec/builders/contact_inbox_builder_spec.rb @@ -12,8 +12,8 @@ describe ::ContactInboxBuilder do it 'does not create contact inbox when contact inbox already exists with the source id provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: twilio_inbox, source_id: contact.phone_number) contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: twilio_inbox.id, + contact: contact, + inbox: twilio_inbox, source_id: contact.phone_number ).perform @@ -23,8 +23,8 @@ describe ::ContactInboxBuilder do it 'does not create contact inbox when contact inbox already exists with phone number and source id is not provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: twilio_inbox, source_id: contact.phone_number) contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: twilio_inbox.id + contact: contact, + inbox: twilio_inbox ).perform expect(contact_inbox.id).to eq(existing_contact_inbox.id) @@ -33,8 +33,8 @@ describe ::ContactInboxBuilder do it 'creates a new contact inbox when different source id is provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: twilio_inbox, source_id: contact.phone_number) contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: twilio_inbox.id, + contact: contact, + inbox: twilio_inbox, source_id: '+224213223422' ).perform @@ -44,12 +44,23 @@ describe ::ContactInboxBuilder do it 'creates a contact inbox with contact phone number when source id not provided and no contact inbox exists' do contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: twilio_inbox.id + contact: contact, + inbox: twilio_inbox ).perform expect(contact_inbox.source_id).to eq(contact.phone_number) end + + it 'raises error when contact phone number is not present and no source id is provided' do + contact.update!(phone_number: nil) + + expect do + described_class.new( + contact: contact, + inbox: twilio_inbox + ).perform + end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact phone number') + end end describe 'twilio whatsapp inbox' do @@ -59,8 +70,8 @@ describe ::ContactInboxBuilder do it 'does not create contact inbox when contact inbox already exists with the source id provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: twilio_inbox, source_id: "whatsapp:#{contact.phone_number}") contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: twilio_inbox.id, + contact: contact, + inbox: twilio_inbox, source_id: "whatsapp:#{contact.phone_number}" ).perform @@ -70,8 +81,8 @@ describe ::ContactInboxBuilder do it 'does not create contact inbox when contact inbox already exists with phone number and source id is not provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: twilio_inbox, source_id: "whatsapp:#{contact.phone_number}") contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: twilio_inbox.id + contact: contact, + inbox: twilio_inbox ).perform expect(contact_inbox.id).to eq(existing_contact_inbox.id) @@ -80,8 +91,8 @@ describe ::ContactInboxBuilder do it 'creates a new contact inbox when different source id is provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: twilio_inbox, source_id: "whatsapp:#{contact.phone_number}") contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: twilio_inbox.id, + contact: contact, + inbox: twilio_inbox, source_id: 'whatsapp:+555555' ).perform @@ -91,12 +102,23 @@ describe ::ContactInboxBuilder do it 'creates a contact inbox with contact phone number when source id not provided and no contact inbox exists' do contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: twilio_inbox.id + contact: contact, + inbox: twilio_inbox ).perform expect(contact_inbox.source_id).to eq("whatsapp:#{contact.phone_number}") end + + it 'raises error when contact phone number is not present and no source id is provided' do + contact.update!(phone_number: nil) + + expect do + described_class.new( + contact: contact, + inbox: twilio_inbox + ).perform + end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact phone number') + end end describe 'whatsapp inbox' do @@ -105,8 +127,8 @@ describe ::ContactInboxBuilder do it 'does not create contact inbox when contact inbox already exists with the source id provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: whatsapp_inbox, source_id: contact.phone_number&.delete('+')) contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: whatsapp_inbox.id, + contact: contact, + inbox: whatsapp_inbox, source_id: contact.phone_number&.delete('+') ).perform @@ -116,8 +138,8 @@ describe ::ContactInboxBuilder do it 'does not create contact inbox when contact inbox already exists with phone number and source id is not provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: whatsapp_inbox, source_id: contact.phone_number&.delete('+')) contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: whatsapp_inbox.id + contact: contact, + inbox: whatsapp_inbox ).perform expect(contact_inbox.id).to be(existing_contact_inbox.id) @@ -126,8 +148,8 @@ describe ::ContactInboxBuilder do it 'creates a new contact inbox when different source id is provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: whatsapp_inbox, source_id: contact.phone_number&.delete('+')) contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: whatsapp_inbox.id, + contact: contact, + inbox: whatsapp_inbox, source_id: '555555' ).perform @@ -137,12 +159,23 @@ describe ::ContactInboxBuilder do it 'creates a contact inbox with contact phone number when source id not provided and no contact inbox exists' do contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: whatsapp_inbox.id + contact: contact, + inbox: whatsapp_inbox ).perform expect(contact_inbox.source_id).to eq(contact.phone_number&.delete('+')) end + + it 'raises error when contact phone number is not present and no source id is provided' do + contact.update!(phone_number: nil) + + expect do + described_class.new( + contact: contact, + inbox: whatsapp_inbox + ).perform + end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact phone number') + end end describe 'sms inbox' do @@ -152,8 +185,8 @@ describe ::ContactInboxBuilder do it 'does not create contact inbox when contact inbox already exists with the source id provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: sms_inbox, source_id: contact.phone_number) contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: sms_inbox.id, + contact: contact, + inbox: sms_inbox, source_id: contact.phone_number ).perform @@ -163,8 +196,8 @@ describe ::ContactInboxBuilder do it 'does not create contact inbox when contact inbox already exists with phone number and source id is not provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: sms_inbox, source_id: contact.phone_number) contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: sms_inbox.id + contact: contact, + inbox: sms_inbox ).perform expect(contact_inbox.id).to eq(existing_contact_inbox.id) @@ -173,8 +206,8 @@ describe ::ContactInboxBuilder do it 'creates a new contact inbox when different source id is provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: sms_inbox, source_id: contact.phone_number) contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: sms_inbox.id, + contact: contact, + inbox: sms_inbox, source_id: '+224213223422' ).perform @@ -184,12 +217,23 @@ describe ::ContactInboxBuilder do it 'creates a contact inbox with contact phone number when source id not provided and no contact inbox exists' do contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: sms_inbox.id + contact: contact, + inbox: sms_inbox ).perform expect(contact_inbox.source_id).to eq(contact.phone_number) end + + it 'raises error when contact phone number is not present and no source id is provided' do + contact.update!(phone_number: nil) + + expect do + described_class.new( + contact: contact, + inbox: sms_inbox + ).perform + end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact phone number') + end end describe 'email inbox' do @@ -199,8 +243,8 @@ describe ::ContactInboxBuilder do it 'does not create contact inbox when contact inbox already exists with the source id provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: email_inbox, source_id: contact.email) contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: email_inbox.id, + contact: contact, + inbox: email_inbox, source_id: contact.email ).perform @@ -210,8 +254,8 @@ describe ::ContactInboxBuilder do it 'does not create contact inbox when contact inbox already exists with email and source id is not provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: email_inbox, source_id: contact.email) contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: email_inbox.id + contact: contact, + inbox: email_inbox ).perform expect(contact_inbox.id).to eq(existing_contact_inbox.id) @@ -220,8 +264,8 @@ describe ::ContactInboxBuilder do it 'creates a new contact inbox when different source id is provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: email_inbox, source_id: contact.email) contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: email_inbox.id, + contact: contact, + inbox: email_inbox, source_id: 'xyc@xyc.com' ).perform @@ -231,12 +275,23 @@ describe ::ContactInboxBuilder do it 'creates a contact inbox with contact email when source id not provided and no contact inbox exists' do contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: email_inbox.id + contact: contact, + inbox: email_inbox ).perform expect(contact_inbox.source_id).to eq(contact.email) end + + it 'raises error when contact email is not present and no source id is provided' do + contact.update!(email: nil) + + expect do + described_class.new( + contact: contact, + inbox: email_inbox + ).perform + end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact email') + end end describe 'api inbox' do @@ -246,8 +301,8 @@ describe ::ContactInboxBuilder do it 'does not create contact inbox when contact inbox already exists with the source id provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: api_inbox, source_id: 'test') contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: api_inbox.id, + contact: contact, + inbox: api_inbox, source_id: 'test' ).perform @@ -257,8 +312,8 @@ describe ::ContactInboxBuilder do it 'creates a new contact inbox when different source id is provided' do existing_contact_inbox = create(:contact_inbox, contact: contact, inbox: api_inbox, source_id: SecureRandom.uuid) contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: api_inbox.id, + contact: contact, + inbox: api_inbox, source_id: 'test' ).perform @@ -268,61 +323,12 @@ describe ::ContactInboxBuilder do it 'creates a contact inbox with SecureRandom.uuid when source id not provided and no contact inbox exists' do contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: api_inbox.id + contact: contact, + inbox: api_inbox ).perform expect(contact_inbox.source_id).not_to be_nil end end - - describe 'web widget' do - let!(:website_channel) { create(:channel_widget, account: account) } - let!(:website_inbox) { create(:inbox, channel: website_channel, account: account) } - - it 'does not create contact inbox' do - contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: website_inbox.id, - source_id: 'test' - ).perform - - expect(contact_inbox).to be_nil - end - end - - describe 'facebook inbox' do - before do - stub_request(:post, /graph.facebook.com/) - end - - let!(:facebook_channel) { create(:channel_facebook_page, account: account) } - let!(:facebook_inbox) { create(:inbox, channel: facebook_channel, account: account) } - - it 'does not create contact inbox' do - contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: facebook_inbox.id, - source_id: 'test' - ).perform - - expect(contact_inbox).to be_nil - end - end - - describe 'twitter inbox' do - let!(:twitter_channel) { create(:channel_twitter_profile, account: account) } - let!(:twitter_inbox) { create(:inbox, channel: twitter_channel, account: account) } - - it 'does not create contact inbox' do - contact_inbox = described_class.new( - contact_id: contact.id, - inbox_id: twitter_inbox.id, - source_id: 'test' - ).perform - - expect(contact_inbox).to be_nil - end - end end end diff --git a/spec/builders/contact_builder_spec.rb b/spec/builders/contact_inbox_with_contact_builder_spec.rb similarity index 98% rename from spec/builders/contact_builder_spec.rb rename to spec/builders/contact_inbox_with_contact_builder_spec.rb index 29df0da22..e76d199d4 100644 --- a/spec/builders/contact_builder_spec.rb +++ b/spec/builders/contact_inbox_with_contact_builder_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -describe ::ContactBuilder do +describe ::ContactInboxWithContactBuilder do let(:account) { create(:account) } let(:inbox) { create(:inbox, account: account) } let(:contact) { create(:contact, email: 'xyc@example.com', phone_number: '+23423424123', account: account, identifier: '123') }