Skip to content

Commit

Permalink
Fixed issue #2329 - Unable to import (update) users with email addres…
Browse files Browse the repository at this point in the history
…ses als lookup index written in upper letters.
  • Loading branch information
znuny-robo committed Nov 6, 2018
1 parent 27dfeac commit 13b3b84
Show file tree
Hide file tree
Showing 19 changed files with 289 additions and 42 deletions.
Expand Up @@ -1320,6 +1320,7 @@ class App.Import extends App.ControllerModal
params.set('try', true) params.set('try', true)
if _.isEmpty(params.get('data')) if _.isEmpty(params.get('data'))
params.delete('data') params.delete('data')
@formDisable(e)
@ajax( @ajax(
id: 'csv_import' id: 'csv_import'
type: 'POST' type: 'POST'
Expand All @@ -1341,6 +1342,14 @@ class App.Import extends App.ControllerModal
return return
@data = data @data = data
@update() @update()
@formEnable(e)
error: (data) =>
details = data.responseJSON || {}
@notify
type: 'error'
msg: App.i18n.translateContent(details.error_human || details.error || 'Unable to import!')
timeout: 6000
@formEnable(e)
) )


class App.ImportTryResult extends App.ControllerModal class App.ImportTryResult extends App.ControllerModal
Expand All @@ -1361,10 +1370,23 @@ class App.ImportTryResult extends App.ControllerModal
import_example_url: "#{@baseUrl}/import" import_example_url: "#{@baseUrl}/import"
result: @result result: @result
)) ))

# check if data is processing...
if @data
result = App.view("#{@templateDirectory}/result")(
@data
)
content.find('.js-error').html(result)
if result
content.find('.js-error').removeClass('hide')
else
content.find('.js-error').addClass('hide')

content content


onSubmit: (e) => onSubmit: (e) =>
@params.set('try', false) @params.set('try', false)
@formDisable(e)
@ajax( @ajax(
id: 'csv_import' id: 'csv_import'
type: 'POST' type: 'POST'
Expand All @@ -1386,6 +1408,14 @@ class App.ImportTryResult extends App.ControllerModal
return return
@data = data @data = data
@update() @update()
@formEnable(e)
error: (data) =>
details = data.responseJSON || {}
@notify
type: 'error'
msg: App.i18n.translateContent(details.error_human || details.error || 'Unable to import!')
timeout: 6000
@formEnable(e)
) )


class App.ImportResult extends App.ControllerModal class App.ImportResult extends App.ControllerModal
Expand Down
7 changes: 6 additions & 1 deletion app/controllers/organizations_controller.rb
Expand Up @@ -349,7 +349,12 @@ def import_example
# @response_message 401 Invalid session. # @response_message 401 Invalid session.
def import_start def import_start
permission_check('admin.user') permission_check('admin.user')
string = params[:data] || params[:file].read.force_encoding('utf-8') string = params[:data]
if string.blank? && params[:file].present?
string = params[:file].read.force_encoding('utf-8')
end
raise Exceptions::UnprocessableEntity, 'No source data submitted!' if string.blank?

result = Organization.csv_import( result = Organization.csv_import(
string: string, string: string,
parse_params: { parse_params: {
Expand Down
7 changes: 6 additions & 1 deletion app/controllers/text_modules_controller.rb
Expand Up @@ -186,7 +186,12 @@ def import_example
# @response_message 401 Invalid session. # @response_message 401 Invalid session.
def import_start def import_start
permission_check('admin.text_module') permission_check('admin.text_module')
string = params[:data] || params[:file].read.force_encoding('utf-8') string = params[:data]
if string.blank? && params[:file].present?
string = params[:file].read.force_encoding('utf-8')
end
raise Exceptions::UnprocessableEntity, 'No source data submitted!' if string.blank?

result = TextModule.csv_import( result = TextModule.csv_import(
string: string, string: string,
parse_params: { parse_params: {
Expand Down
8 changes: 7 additions & 1 deletion app/controllers/ticket_articles_controller.rb
Expand Up @@ -318,8 +318,14 @@ def import_start
raise 'Only can import tickets if system is in import mode.' raise 'Only can import tickets if system is in import mode.'
end end


string = params[:data]
if string.blank? && params[:file].present?
string = params[:file].read.force_encoding('utf-8')
end
raise Exceptions::UnprocessableEntity, 'No source data submitted!' if string.blank?

result = Ticket::Article.csv_import( result = Ticket::Article.csv_import(
string: params[:file].read.force_encoding('utf-8'), string: string,
parse_params: { parse_params: {
col_sep: ';', col_sep: ';',
}, },
Expand Down
7 changes: 6 additions & 1 deletion app/controllers/tickets_controller.rb
Expand Up @@ -641,7 +641,12 @@ def import_start
raise 'Only can import tickets if system is in import mode.' raise 'Only can import tickets if system is in import mode.'
end end


string = params[:data] || params[:file].read.force_encoding('utf-8') string = params[:data]
if string.blank? && params[:file].present?
string = params[:file].read.force_encoding('utf-8')
end
raise Exceptions::UnprocessableEntity, 'No source data submitted!' if string.blank?

result = Ticket.csv_import( result = Ticket.csv_import(
string: string, string: string,
parse_params: { parse_params: {
Expand Down
7 changes: 6 additions & 1 deletion app/controllers/users_controller.rb
Expand Up @@ -1043,7 +1043,12 @@ def import_example
# @response_message 401 Invalid session. # @response_message 401 Invalid session.
def import_start def import_start
permission_check('admin.user') permission_check('admin.user')
string = params[:data] || params[:file].read.force_encoding('utf-8') string = params[:data]
if string.blank? && params[:file].present?
string = params[:file].read.force_encoding('utf-8')
end
raise Exceptions::UnprocessableEntity, 'No source data submitted!' if string.blank?

result = User.csv_import( result = User.csv_import(
string: string, string: string,
parse_params: { parse_params: {
Expand Down
17 changes: 15 additions & 2 deletions app/models/application_model/can_lookup.rb
Expand Up @@ -20,20 +20,33 @@ module ApplicationModel::CanLookup
=end =end


def lookup(**attr) def lookup(**attr)
raise ArgumentError, "Multiple lookup attributes given (#{attr.keys.join(', ')}), only support (#{lookup_keys.join(', ')})" if attr.many?

attr.transform_keys!(&:to_sym).slice!(*lookup_keys) attr.transform_keys!(&:to_sym).slice!(*lookup_keys)
raise ArgumentError, "Valid lookup attribute required (#{lookup_keys.join(', ')})" if attr.empty? raise ArgumentError, "Valid lookup attribute required (#{lookup_keys.join(', ')})" if attr.empty?
raise ArgumentError, "Multiple lookup attributes given (#{attr.keys.join(', ')})" if attr.many?


record = cache_get(attr.values.first) record = cache_get(attr.values.first)
record ||= find_and_save_to_cache_by(attr) record ||= find_and_save_to_cache_by(attr)
end end


private =begin
return possible lookup keys for model
result = Model.lookup_keys
returns
[:id, :name] # or fror users [:id, :login, :email]
=end


def lookup_keys def lookup_keys
@lookup_keys ||= %i[id name login email number] & attribute_names.map(&:to_sym) @lookup_keys ||= %i[id name login email number] & attribute_names.map(&:to_sym)
end end


private

def find_and_save_to_cache_by(attr) def find_and_save_to_cache_by(attr)
if !Rails.application.config.db_case_sensitive && string_key?(attr.keys.first) if !Rails.application.config.db_case_sensitive && string_key?(attr.keys.first)
where(attr).find { |r| r[attr.keys.first] == attr.values.first } where(attr).find { |r| r[attr.keys.first] == attr.values.first }
Expand Down
35 changes: 28 additions & 7 deletions app/models/concerns/can_csv_import.rb
Expand Up @@ -118,6 +118,17 @@ def csv_import(data)
return result return result
end end


# check if min one lookup key exists
if header.count == (header - lookup_keys.map(&:to_s)).count
errors.push "No lookup column like #{lookup_keys.map(&:to_s).join(',')} for #{new.class} found."
result = {
errors: errors,
try: try,
result: 'failed',
}
return result
end

# get payload based on csv # get payload based on csv
payload = [] payload = []
rows.each do |row| rows.each do |row|
Expand Down Expand Up @@ -172,11 +183,15 @@ def csv_import(data)
payload.each do |attributes| payload.each do |attributes|
line_count += 1 line_count += 1
record = nil record = nil
%i[id number name login email].each do |lookup_by| lookup_keys.each do |lookup_by|
next if !attributes[lookup_by] next if attributes[lookup_by].blank?


params = {} params = {}
params[lookup_by] = attributes[lookup_by] params[lookup_by] = if %i[email login].include?(lookup_by)
attributes[lookup_by].downcase
else
attributes[lookup_by]
end
record = lookup(params) record = lookup(params)
break if record break if record
end end
Expand All @@ -199,6 +214,7 @@ def csv_import(data)
end end


# create object # create object
BulkImportInfo.enable
Transaction.execute(disable_notification: true, reset_user_id: true) do Transaction.execute(disable_notification: true, reset_user_id: true) do
UserInfo.current_user_id = clean_params[:updated_by_id] || clean_params[:created_by_id] UserInfo.current_user_id = clean_params[:updated_by_id] || clean_params[:created_by_id]
if !record || delete == true if !record || delete == true
Expand All @@ -217,7 +233,7 @@ def csv_import(data)
record.associations_from_param(attributes) record.associations_from_param(attributes)
record.save! record.save!
rescue => e rescue => e
errors.push "Line #{line_count}: #{e.message}" errors.push "Line #{line_count}: Unable to create record - #{e.message}"
next next
end end
else else
Expand All @@ -234,14 +250,20 @@ def csv_import(data)


record.with_lock do record.with_lock do
record.associations_from_param(attributes) record.associations_from_param(attributes)
record.update!(clean_params) clean_params.each do |key, value|
record[key] = value
end
next if !record.changed?

record.save!
end end
rescue => e rescue => e
errors.push "Line #{line_count}: #{e.message}" errors.push "Line #{line_count}: Unable to update record - #{e.message}"
next next
end end
end end
end end
BulkImportInfo.disable


records.push record records.push record
end end
Expand All @@ -258,7 +280,6 @@ def csv_import(data)
try: try, try: try,
result: result, result: result,
} }

end end


=begin =begin
Expand Down
3 changes: 3 additions & 0 deletions app/models/cti/caller_id.rb
Expand Up @@ -9,6 +9,9 @@ class CallerId < ApplicationModel
after_commit :update_cti_logs, on: :destroy after_commit :update_cti_logs, on: :destroy
after_commit :update_cti_logs_with_fg_optimization, on: :create after_commit :update_cti_logs_with_fg_optimization, on: :create


skip_callback :commit, :after, :update_cti_logs, if: -> { BulkImportInfo.enabled? }
skip_callback :commit, :after, :update_cti_logs_with_fg_optimization, if: -> { BulkImportInfo.enabled? }

=begin =begin
Cti::CallerId.maybe_add( Cti::CallerId.maybe_add(
Expand Down
15 changes: 9 additions & 6 deletions app/models/user.rb
Expand Up @@ -21,12 +21,15 @@ class User < ApplicationModel


before_validation :check_name, :check_email, :check_login, :ensure_uniq_email, :ensure_password, :ensure_roles, :ensure_identifier before_validation :check_name, :check_email, :check_login, :ensure_uniq_email, :ensure_password, :ensure_roles, :ensure_identifier
before_validation :check_mail_delivery_failed, on: :update before_validation :check_mail_delivery_failed, on: :update
before_create :check_preferences_default, :validate_preferences, :validate_ooo, :domain_based_assignment, :set_locale before_create :check_preferences_default, :validate_preferences, :validate_ooo, :domain_based_assignment, :set_locale
before_update :check_preferences_default, :validate_preferences, :validate_ooo, :reset_login_failed, :validate_agent_limit_by_attributes, :last_admin_check_by_attribute before_update :check_preferences_default, :validate_preferences, :validate_ooo, :reset_login_failed, :validate_agent_limit_by_attributes, :last_admin_check_by_attribute
after_create :avatar_for_email_check after_create :avatar_for_email_check
after_update :avatar_for_email_check after_update :avatar_for_email_check
after_commit :update_caller_id after_commit :update_caller_id
before_destroy :destroy_longer_required_objects before_destroy :destroy_longer_required_objects

skip_callback :create, :after, :avatar_for_email_check, if: -> { BulkImportInfo.enabled? }
skip_callback :update, :after, :avatar_for_email_check, if: -> { BulkImportInfo.enabled? }


store :preferences store :preferences


Expand Down
13 changes: 13 additions & 0 deletions lib/bulk_import_info.rb
@@ -0,0 +1,13 @@
module BulkImportInfo
def self.enabled?
Thread.current[:bulk_import]
end

def self.enable
Thread.current[:bulk_import] = true
end

def self.disable
Thread.current[:bulk_import] = false
end
end
11 changes: 8 additions & 3 deletions spec/models/concerns/can_lookup_examples.rb
Expand Up @@ -4,8 +4,8 @@
describe '::lookup' do describe '::lookup' do
let(:subject) { described_class } let(:subject) { described_class }
let(:ensure_instance) { create(subject.to_s.downcase) if subject.none? } let(:ensure_instance) { create(subject.to_s.downcase) if subject.none? }
let(:string_attributes) { (%i[name login email number] & subject.attribute_names.map(&:to_sym)) } let(:string_attributes) { subject.lookup_keys }
let(:non_attributes) { (%i[id name login email number] - subject.attribute_names.map(&:to_sym)) } let(:non_attributes) { (subject.attribute_names.map(&:to_sym) - subject.lookup_keys) }
let(:lookup_id) { 1 } let(:lookup_id) { 1 }
let(:cache_key) { "#{subject}::#{lookup_id}" } let(:cache_key) { "#{subject}::#{lookup_id}" }
let(:cache_expiry_param) { { expires_in: 7.days } } let(:cache_expiry_param) { { expires_in: 7.days } }
Expand Down Expand Up @@ -35,7 +35,7 @@
end end


it 'accepts exactly one attribute-value pair' do it 'accepts exactly one attribute-value pair' do
expect { subject.lookup(:id => lookup_id, string_attributes.sample => 'foo') } expect { subject.lookup(id: lookup_id, some_attribute: 'foo') }
.to raise_error(ArgumentError) .to raise_error(ArgumentError)
end end


Expand Down Expand Up @@ -69,5 +69,10 @@
.to have_received(:read) .to have_received(:read)
.with(cache_key) .with(cache_key)
end end

it 'verify lookup keys' do
expect(subject.lookup_keys).to match_array((%i[id name login email number] & subject.attribute_names.map(&:to_sym)))
end

end end
end end
4 changes: 2 additions & 2 deletions spec/requests/organization_spec.rb
Expand Up @@ -512,8 +512,8 @@
expect(json_response['records'].count).to eq(2) expect(json_response['records'].count).to eq(2)
expect(json_response['result']).to eq('failed') expect(json_response['result']).to eq('failed')
expect(json_response['errors'].count).to eq(2) expect(json_response['errors'].count).to eq(2)
expect(json_response['errors'][0]).to eq("Line 1: unknown attribute 'name2' for Organization.") expect(json_response['errors'][0]).to eq("Line 1: Unable to create record - unknown attribute 'name2' for Organization.")
expect(json_response['errors'][1]).to eq("Line 2: unknown attribute 'name2' for Organization.") expect(json_response['errors'][1]).to eq("Line 2: Unable to create record - unknown attribute 'name2' for Organization.")


# valid file try # valid file try
csv_file_path = Rails.root.join('test', 'data', 'csv', 'organization_simple.csv') csv_file_path = Rails.root.join('test', 'data', 'csv', 'organization_simple.csv')
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/text_module_spec.rb
Expand Up @@ -59,8 +59,8 @@
expect(json_response['records'].count).to eq(2) expect(json_response['records'].count).to eq(2)
expect(json_response['result']).to eq('failed') expect(json_response['result']).to eq('failed')
expect(json_response['errors'].count).to eq(2) expect(json_response['errors'].count).to eq(2)
expect(json_response['errors'][0]).to eq("Line 1: unknown attribute 'keywords2' for TextModule.") expect(json_response['errors'][0]).to eq("Line 1: Unable to create record - unknown attribute 'keywords2' for TextModule.")
expect(json_response['errors'][1]).to eq("Line 2: unknown attribute 'keywords2' for TextModule.") expect(json_response['errors'][1]).to eq("Line 2: Unable to create record - unknown attribute 'keywords2' for TextModule.")


# valid file try # valid file try
csv_file_path = Rails.root.join('test', 'data', 'csv', 'text_module_simple.csv') csv_file_path = Rails.root.join('test', 'data', 'csv', 'text_module_simple.csv')
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/user_spec.rb
Expand Up @@ -872,8 +872,8 @@
expect(json_response['records'].count).to eq(2) expect(json_response['records'].count).to eq(2)
expect(json_response['result']).to eq('failed') expect(json_response['result']).to eq('failed')
expect(json_response['errors'].count).to eq(2) expect(json_response['errors'].count).to eq(2)
expect(json_response['errors'][0]).to eq("Line 1: unknown attribute 'firstname2' for User.") expect(json_response['errors'][0]).to eq("Line 1: Unable to create record - unknown attribute 'firstname2' for User.")
expect(json_response['errors'][1]).to eq("Line 2: unknown attribute 'firstname2' for User.") expect(json_response['errors'][1]).to eq("Line 2: Unable to create record - unknown attribute 'firstname2' for User.")


# valid file try # valid file try
csv_file_path = Rails.root.join('test', 'data', 'csv', 'user_simple.csv') csv_file_path = Rails.root.join('test', 'data', 'csv', 'user_simple.csv')
Expand Down

0 comments on commit 13b3b84

Please sign in to comment.