chore: Contact import improvements (#7787)

- Ensure existing contact information is updated on data import
- Refactor the existing job to make it more readable
- Fixes issues with import files in the wrong encoding
fixes: #7307

app/javascript/dashboard/i18n/locale/en/contact.json

@@ -71,7 +71,7 @@
       "SUBMIT": "Import",
       "CANCEL": "Cancel"
     },
-    "SUCCESS_MESSAGE": "Contacts saved successfully",
+    "SUCCESS_MESSAGE": "You will be notified via email when the import is complete.",
     "ERROR_MESSAGE": "There was an error, please try again"
   },
   "EXPORT_CONTACTS": {

app/jobs/data_import_job.rb

@@ -6,15 +6,15 @@ class DataImportJob < ApplicationJob
 
   def perform(data_import)
     @data_import = data_import
+    @contact_manager = DataImport::ContactManager.new(@data_import.account)
     process_import_file
-    send_failed_records_to_admin
+    send_import_notification_to_admin
   end
 
   private
 
   def process_import_file
     @data_import.update!(status: :processing)
-
     contacts, rejected_contacts = parse_csv_and_build_contacts
 
     import_contacts(contacts)
@@ -25,21 +25,28 @@ class DataImportJob < ApplicationJob
   def parse_csv_and_build_contacts
     contacts = []
     rejected_contacts = []
-    csv = CSV.parse(@data_import.import_file.download, headers: true)
+    # Ensuring that importing non utf-8 characters will not throw error
+    data = @data_import.import_file.download
+    clean_data = data.encode('UTF-8', 'binary', invalid: :replace, undef: :replace, replace: '')
+    csv = CSV.parse(clean_data, headers: true)
 
     csv.each do |row|
-      current_contact = build_contact(row.to_h.with_indifferent_access, @data_import.account)
+      current_contact = @contact_manager.build_contact(row.to_h.with_indifferent_access)
       if current_contact.valid?
         contacts << current_contact
       else
-        row['errors'] = current_contact.errors.full_messages.join(', ')
-        rejected_contacts << row
+        append_rejected_contact(row, current_contact, rejected_contacts)
       end
     end
 
     [contacts, rejected_contacts]
   end
 
+  def append_rejected_contact(row, contact, rejected_contacts)
+    row['errors'] = contact.errors.full_messages.join(', ')
+    rejected_contacts << row
+  end
+
   def import_contacts(contacts)
     # <struct ActiveRecord::Import::Result failed_instances=[], num_inserts=1, ids=[444, 445], results=[]>
     Contact.import(contacts, synchronize: contacts, on_duplicate_key_ignore: true, track_validation_failures: true, validate: true, batch_size: 1000)
@@ -49,63 +56,13 @@ class DataImportJob < ApplicationJob
     @data_import.update!(status: :completed, processed_records: processed_records, total_records: processed_records + rejected_records)
   end
 
-  def build_contact(params, account)
-    contact = find_or_initialize_contact(params, account)
-    contact.name = params[:name] if params[:name].present?
-    contact.additional_attributes ||= {}
-    contact.additional_attributes[:company] = params[:company] if params[:company].present?
-    contact.additional_attributes[:city] = params[:city] if params[:city].present?
-    contact.assign_attributes(custom_attributes: contact.custom_attributes.merge(params.except(:identifier, :email, :name, :phone_number)))
-    contact
-  end
-
-  def find_or_initialize_contact(params, account)
-    contact = find_existing_contact(params, account)
-    contact ||= account.contacts.new(params.slice(:email, :identifier, :phone_number))
-    contact
-  end
-
-  def find_existing_contact(params, account)
-    contact = find_contact_by_identifier(params, account)
-    contact ||= find_contact_by_email(params, account)
-    contact ||= find_contact_by_phone_number(params, account)
-
-    update_contact_with_merged_attributes(params, contact) if contact.present? && contact.valid?
-    contact
-  end
-
-  def find_contact_by_identifier(params, account)
-    return unless params[:identifier]
-
-    account.contacts.find_by(identifier: params[:identifier])
-  end
-
-  def find_contact_by_email(params, account)
-    return unless params[:email]
-
-    account.contacts.find_by(email: params[:email])
-  end
-
-  def find_contact_by_phone_number(params, account)
-    return unless params[:phone_number]
-
-    account.contacts.find_by(phone_number: params[:phone_number])
-  end
-
-  def update_contact_with_merged_attributes(params, contact)
-    contact.email = params[:email] if params[:email].present?
-    contact.phone_number = params[:phone_number] if params[:phone_number].present?
-    contact.save
-  end
-
   def save_failed_records_csv(rejected_contacts)
     csv_data = generate_csv_data(rejected_contacts)
-
     return if csv_data.blank?
 
     @data_import.failed_records.attach(io: StringIO.new(csv_data), filename: "#{Time.zone.today.strftime('%Y%m%d')}_contacts.csv",
                                        content_type: 'text/csv')
-    send_failed_records_to_admin
+    send_import_notification_to_admin
   end
 
   def generate_csv_data(rejected_contacts)
@@ -121,7 +78,7 @@ class DataImportJob < ApplicationJob
     end
   end
 
-  def send_failed_records_to_admin
-    AdministratorNotifications::ChannelNotificationsMailer.with(account: @data_import.account).failed_records(@data_import).deliver_later
+  def send_import_notification_to_admin
+    AdministratorNotifications::ChannelNotificationsMailer.with(account: @data_import.account).contact_import_complete(@data_import).deliver_later
   end
 end

app/mailers/administrator_notifications/channel_notifications_mailer.rb

@@ -1,16 +1,4 @@
 class AdministratorNotifications::ChannelNotificationsMailer < ApplicationMailer
-  def failed_records(resource)
-    return unless smtp_config_set_or_development?
-
-    subject = 'Contact Import Completed'
-
-    @attachment_url = Rails.application.routes.url_helpers.rails_blob_url(resource.failed_records) if resource.failed_records.attached?
-    @action_url = "#{ENV.fetch('FRONTEND_URL', nil)}/app/accounts/#{resource.account.id}/contacts"
-    @failed_contacts = resource.total_records - resource.processed_records
-    @imported_contacts = resource.processed_records
-    send_mail_with_liquid(to: admin_emails, subject: subject) and return
-  end
-
   def slack_disconnect
     return unless smtp_config_set_or_development?
 
@@ -43,6 +31,19 @@ class AdministratorNotifications::ChannelNotificationsMailer < ApplicationMailer
     send_mail_with_liquid(to: admin_emails, subject: subject) and return
   end
 
+  def contact_import_complete(resource)
+    return unless smtp_config_set_or_development?
+
+    subject = 'Contact Import Completed'
+
+    @action_url = Rails.application.routes.url_helpers.rails_blob_url(resource.failed_records) if resource.failed_records.attached?
+    @action_url ||= "#{ENV.fetch('FRONTEND_URL', nil)}/app/accounts/#{resource.account.id}/contacts"
+    @meta = {}
+    @meta['failed_contacts'] = resource.total_records - resource.processed_records
+    @meta['imported_contacts'] = resource.processed_records
+    send_mail_with_liquid(to: admin_emails, subject: subject) and return
+  end
+
   def contact_export_complete(file_url)
     return unless smtp_config_set_or_development?
 
@@ -56,4 +57,8 @@ class AdministratorNotifications::ChannelNotificationsMailer < ApplicationMailer
   def admin_emails
     Current.account.administrators.pluck(:email)
   end
+
+  def liquid_locals
+    super.merge({ meta: @meta })
+  end
 end

app/services/data_import/contact_manager.rb

@@ -0,0 +1,68 @@
+class DataImport::ContactManager
+  def initialize(account)
+    @account = account
+  end
+
+  def build_contact(params)
+    contact = find_or_initialize_contact(params)
+    update_contact_attributes(params, contact)
+    contact
+  end
+
+  def find_or_initialize_contact(params)
+    contact = find_existing_contact(params)
+    contact_params = params.slice(:email, :identifier, :phone_number)
+    contact_params[:phone_number] = format_phone_number(contact_params[:phone_number]) if contact_params[:phone_number].present?
+    contact ||= @account.contacts.new(contact_params)
+    contact
+  end
+
+  def find_existing_contact(params)
+    contact = find_contact_by_identifier(params)
+    contact ||= find_contact_by_email(params)
+    contact ||= find_contact_by_phone_number(params)
+
+    update_contact_with_merged_attributes(params, contact) if contact.present? && contact.valid?
+    contact
+  end
+
+  def find_contact_by_identifier(params)
+    return unless params[:identifier]
+
+    @account.contacts.find_by(identifier: params[:identifier])
+  end
+
+  def find_contact_by_email(params)
+    return unless params[:email]
+
+    @account.contacts.find_by(email: params[:email])
+  end
+
+  def find_contact_by_phone_number(params)
+    return unless params[:phone_number]
+
+    @account.contacts.find_by(phone_number: format_phone_number(params[:phone_number]))
+  end
+
+  def format_phone_number(phone_number)
+    phone_number.start_with?('+') ? phone_number : "+#{phone_number}"
+  end
+
+  def update_contact_with_merged_attributes(params, contact)
+    contact.identifier = params[:identifier] if params[:identifier].present?
+    contact.email = params[:email] if params[:email].present?
+    contact.phone_number = format_phone_number(params[:phone_number]) if params[:phone_number].present?
+    update_contact_attributes(params, contact)
+    contact.save
+  end
+
+  private
+
+  def update_contact_attributes(params, contact)
+    contact.name = params[:name] if params[:name].present?
+    contact.additional_attributes ||= {}
+    contact.additional_attributes[:company] = params[:company] if params[:company].present?
+    contact.additional_attributes[:city] = params[:city] if params[:city].present?
+    contact.assign_attributes(custom_attributes: contact.custom_attributes.merge(params.except(:identifier, :email, :name, :phone_number)))
+  end
+end

app/views/mailers/administrator_notifications/channel_notifications_mailer/contact_import_complete.liquid

@@ -0,0 +1,17 @@
+<p>Hello,</p>
+
+<p>Your contact import has been completed. Please check the contacts tab to view the imported contacts.</p>
+
+<p>Number of records imported: {{meta['imported_contacts']}}</p>
+
+<p>Number of records failed: {{meta['failed_contacts']}}</p>
+
+{% if meta['failed_contacts'] == 0 %}
+  <p>
+    Click <a href="{{action_url}}">here</a> to view the imported contacts.
+  </p>
+{% else %}
+  <p>
+  Click <a href="{{ action_url }}" target="_blank">here</a> to view failed records.
+  </p>
+{% endif %}

app/views/mailers/administrator_notifications/channel_notifications_mailer/failed_records.liquid

@@ -1,14 +0,0 @@
-<p>Hello,</p>
-
-<p>Your contact import has been completed. Please check the contacts tab to view the imported contacts.</p>
-
-<p>Number of records imported. : {{imported_contacts}}</p>
-
-<p>Number of records failed. : {{failed_contacts}}</p>
-
-<p>
-Attachment [<a href="{{ attachment_url }}" _target="blank">Click here to view</a>]
-</p>
-<p>
-Click <a href="{{action_url}}">here</a>
-</p>