From 13be41c844597150b3cf8a4c851f35f8dd89db3f Mon Sep 17 00:00:00 2001 From: Julian Kornberger Date: Fri, 2 Dec 2016 12:09:17 +0100 Subject: [PATCH 1/3] Refactor Cti::CallerId - execute rebuild in a single transaction - reduce queries - DRY up code - satisfy RuboCop --- app/models/cti/caller_id.rb | 69 +++++++++++++------------------------ 1 file changed, 24 insertions(+), 45 deletions(-) diff --git a/app/models/cti/caller_id.rb b/app/models/cti/caller_id.rb index 4118a58df..71fe4291f 100644 --- a/app/models/cti/caller_id.rb +++ b/app/models/cti/caller_id.rb @@ -16,22 +16,17 @@ module Cti =end def self.maybe_add(data) - records = Cti::CallerId.where( + record = find_or_initialize_by( caller_id: data[:caller_id], - level: data[:level], - object: data[:object], - o_id: data[:o_id], - user_id: data[:user_id], - ) - return if records[0] - Cti::CallerId.create( - caller_id: data[:caller_id], - comment: data[:comment], - level: data[:level], - object: data[:object], - o_id: data[:o_id], - user_id: data[:user_id], + level: data[:level], + object: data[:object], + o_id: data[:o_id], + user_id: data[:user_id], ) + if record.new_record? + record.comment = data[:comment] + record.save! + end end =begin @@ -45,28 +40,11 @@ returns =end def self.lookup(caller_id) - result = Cti::CallerId.where( - caller_id: caller_id, - level: 'known', - ).group(:user_id, :id).order(id: 'DESC').limit(20) - if !result[0] - result = Cti::CallerId.where( - caller_id: caller_id, - level: 'maybe', - ).group(:user_id, :id).order(id: 'DESC').limit(20) - end - if !result[0] - result = Cti::CallerId.where( - caller_id: caller_id, - ).order('id DESC').limit(20) - end - - # in case do lookups in external sources - if !result[0] - # ... - end - - result + where(caller_id: caller_id) + .group(:user_id, :id) \ + # first known, then maybe, last others + .order("level != 'known', level != 'maybe', id DESC") + .limit(20) end =begin @@ -143,15 +121,16 @@ returns =end def self.rebuild - Cti::CallerId.delete_all - map = config - map.each { |item| - level = item[:level] - model = item[:model] - item[:model].find_each(batch_size: 500) do |record| - build_item(record, model, level) - end - } + transaction do + delete_all + config.each { |item| + level = item[:level] + model = item[:model] + item[:model].find_each(batch_size: 500) do |record| + build_item(record, model, level) + end + } + end end =begin From ce436c6f01aad4fad37e185af84c72814e7f771b Mon Sep 17 00:00:00 2001 From: Thorsten Date: Mon, 5 Dec 2016 10:19:18 +0100 Subject: [PATCH 2/3] Use guard clause --- app/models/cti/caller_id.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/models/cti/caller_id.rb b/app/models/cti/caller_id.rb index 71fe4291f..e591f5a98 100644 --- a/app/models/cti/caller_id.rb +++ b/app/models/cti/caller_id.rb @@ -23,10 +23,9 @@ module Cti o_id: data[:o_id], user_id: data[:user_id], ) - if record.new_record? - record.comment = data[:comment] - record.save! - end + return if !record.new_record? + record.comment = data[:comment] + record.save! end =begin From fd1b0f9c18670e851ed3280d8978328e8d4ba4bd Mon Sep 17 00:00:00 2001 From: Thorsten Date: Mon, 5 Dec 2016 13:30:55 +0100 Subject: [PATCH 3/3] maybe_add should always return the record --- app/models/cti/caller_id.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/cti/caller_id.rb b/app/models/cti/caller_id.rb index e591f5a98..42e81391e 100644 --- a/app/models/cti/caller_id.rb +++ b/app/models/cti/caller_id.rb @@ -23,7 +23,7 @@ module Cti o_id: data[:o_id], user_id: data[:user_id], ) - return if !record.new_record? + return record if !record.new_record? record.comment = data[:comment] record.save! end