From be115dbb5cff1a34161c500c0b5a2c0623daba4d Mon Sep 17 00:00:00 2001 From: Rolf Schmidt Date: Wed, 12 May 2021 10:31:00 +0000 Subject: [PATCH] Fixes #3550 - CTI Log performance bad because of reformatting of the phone number. --- .../app/controllers/_integration/cti.coffee | 28 +++++++++++++++++ .../app/views/integration/cti.jst.eco | 11 ++++--- app/models/cti/log.rb | 30 +++++++++++-------- .../20210510092410_issue_3550_set_pretty.rb | 12 ++++++++ spec/db/migrate/issue_3550_set_pretty_spec.rb | 19 ++++++++++++ .../update_cti_logs_by_caller_job_spec.rb | 6 ++-- spec/models/cti/log_spec.rb | 12 ++++++++ spec/models/user_spec.rb | 12 ++++---- 8 files changed, 105 insertions(+), 25 deletions(-) create mode 100644 db/migrate/20210510092410_issue_3550_set_pretty.rb create mode 100644 spec/db/migrate/issue_3550_set_pretty_spec.rb diff --git a/app/assets/javascripts/app/controllers/_integration/cti.coffee b/app/assets/javascripts/app/controllers/_integration/cti.coffee index ca2e3038f..c31d90af1 100644 --- a/app/assets/javascripts/app/controllers/_integration/cti.coffee +++ b/app/assets/javascripts/app/controllers/_integration/cti.coffee @@ -73,6 +73,30 @@ class Form extends App.Controller autofocus: false ) + configure_attributes = [ + { + name: 'view_limit', + display: '', + tag: 'select', + null: false, + options: [ + { name: 60, value: 60 } + { name: 120, value: 120 } + { name: 180, value: 180 } + { name: 240, value: 240 } + { name: 300, value: 300 } + ] + }, + ] + new App.ControllerForm( + el: @$('.js-viewLimit') + model: + configure_attributes: configure_attributes, + params: + view_limit: @config['view_limit'] + autofocus: false + ) + for row in @config.notify_map configure_attributes = [ { name: 'user_ids', display: '', tag: 'column_select', multiple: true, null: true, relation: 'User', sortBy: 'firstname' }, @@ -94,6 +118,10 @@ class Form extends App.Controller default_caller_id = @$('input[name=default_caller_id]').val() config.outbound.default_caller_id = cleanupInput(default_caller_id) + # default view limit + view_limit = @$('select[name=view_limit]').val() + config.view_limit = parseInt(view_limit) + # routing table config.outbound.routing_table = [] @$('.js-outboundRouting .js-row').each(-> diff --git a/app/assets/javascripts/app/views/integration/cti.jst.eco b/app/assets/javascripts/app/views/integration/cti.jst.eco index d48a0b3ea..3c773dba5 100644 --- a/app/assets/javascripts/app/views/integration/cti.jst.eco +++ b/app/assets/javascripts/app/views/integration/cti.jst.eco @@ -76,19 +76,22 @@ -

<%- @T('Default caller id.') %> +

<%- @T('Settings') %>

- +
<%- @T('Default caller id') %> - <%- @T('Note') %> + <%- @T('Name') %> + <%- @T('Description') %>
<%- @T('Default caller id for outbound calls.') %> +
+ <%- @T('Shown records in caller log.') %>
@@ -121,4 +124,4 @@ - \ No newline at end of file + diff --git a/app/models/cti/log.rb b/app/models/cti/log.rb index d537550da..21fdc64c3 100644 --- a/app/models/cti/log.rb +++ b/app/models/cti/log.rb @@ -4,10 +4,12 @@ module Cti self.table_name = 'cti_logs' - store :preferences + store :preferences, accessors: %i[from_pretty to_pretty] validates :state, format: { with: /\A(newCall|answer|hangup)\z/,  message: 'newCall|answer|hangup is allowed' } + before_create :set_pretty + before_update :set_pretty after_commit :push_caller_list_update =begin @@ -327,10 +329,10 @@ returns def self.log_records(current_user) cti_config = Setting.get('cti_config') if cti_config[:notify_map].present? - return Cti::Log.where(queue: queues_of_user(current_user, cti_config)).order(created_at: :desc).limit(60) + return Cti::Log.where(queue: queues_of_user(current_user, cti_config)).order(created_at: :desc).limit(view_limit) end - Cti::Log.order(created_at: :desc).limit(60) + Cti::Log.order(created_at: :desc).limit(view_limit) end =begin @@ -448,7 +450,7 @@ Cti::Log.process( end def self.push_caller_list_update?(record) - list_ids = Cti::Log.order(created_at: :desc).limit(60).pluck(:id) + list_ids = Cti::Log.order(created_at: :desc).limit(view_limit).pluck(:id) return true if list_ids.include?(record.id) false @@ -490,6 +492,10 @@ optional you can put the max oldest chat entries as argument # adds virtual attributes when rendering #to_json # see http://api.rubyonrails.org/classes/ActiveModel/Serialization.html def attributes + if !from_pretty || !to_pretty + set_pretty + end + virtual_attributes = { 'from_pretty' => from_pretty, 'to_pretty' => to_pretty, @@ -498,14 +504,11 @@ optional you can put the max oldest chat entries as argument super.merge(virtual_attributes) end - def from_pretty - parsed = TelephoneNumber.parse(from&.sub(/^\+?/, '+')) - parsed.send(parsed.valid? ? :international_number : :original_number) - end - - def to_pretty - parsed = TelephoneNumber.parse(to&.sub(/^\+?/, '+')) - parsed.send(parsed.valid? ? :international_number : :original_number) + def set_pretty + %i[from to].each do |field| + parsed = TelephoneNumber.parse(send(field)&.sub(/^\+?/, '+')) + preferences[:"#{field}_pretty"] = parsed.send(parsed.valid? ? :international_number : :original_number) + end end =begin @@ -556,5 +559,8 @@ return best customer id of caller log customer_id end + def self.view_limit + Hash(Setting.get('cti_config'))['view_limit'] || 60 + end end end diff --git a/db/migrate/20210510092410_issue_3550_set_pretty.rb b/db/migrate/20210510092410_issue_3550_set_pretty.rb new file mode 100644 index 000000000..91529b71a --- /dev/null +++ b/db/migrate/20210510092410_issue_3550_set_pretty.rb @@ -0,0 +1,12 @@ +class Issue3550SetPretty < ActiveRecord::Migration[5.2] + def change + return if !Setting.exists?(name: 'system_init_done') + + Cti::Log.order(created_at: :desc).limit(300).find_each do |log| + log.set_pretty + log.save! + rescue + Rails.logger.error "Issue3550SetPretty: Failed to migrate id #{log.id} with from '#{log.from}' and to '#{log.to}'" + end + end +end diff --git a/spec/db/migrate/issue_3550_set_pretty_spec.rb b/spec/db/migrate/issue_3550_set_pretty_spec.rb new file mode 100644 index 000000000..43865f9ec --- /dev/null +++ b/spec/db/migrate/issue_3550_set_pretty_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' + +RSpec.describe Issue3550SetPretty, type: :db_migration, db_strategy: :reset do + context 'when cti gets migrated to stored pretty values' do + let!(:cti) { create(:'cti/log') } + + before do + migrate + end + + it 'has from_pretty' do + expect(cti.preferences[:from_pretty]).to eq('+49 30 609854180') + end + + it 'has to_pretty' do + expect(cti.preferences[:to_pretty]).to eq('+49 30 609811111') + end + end +end diff --git a/spec/jobs/update_cti_logs_by_caller_job_spec.rb b/spec/jobs/update_cti_logs_by_caller_job_spec.rb index b79a8c113..636cc6941 100644 --- a/spec/jobs/update_cti_logs_by_caller_job_spec.rb +++ b/spec/jobs/update_cti_logs_by_caller_job_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' RSpec.describe UpdateCtiLogsByCallerJob, type: :job do let(:phone) { '1234567890' } let!(:logs) { create_list(:cti_log, 5, direction: :in, from: phone) } - let(:log_prefs) { logs.each(&:reload).map(&:preferences) } + let(:log_prefs) { logs.each(&:reload).map { |log| log.preferences[:from] } } it 'accepts a phone number' do expect { described_class.perform_now(phone) } @@ -14,7 +14,7 @@ RSpec.describe UpdateCtiLogsByCallerJob, type: :job do it 'updates Cti::Logs from that number with "preferences" => {}' do described_class.perform_now(phone) - expect(log_prefs).to all(be_empty) + expect(log_prefs).to eq(Array.new(5) { nil }) end end @@ -24,7 +24,7 @@ RSpec.describe UpdateCtiLogsByCallerJob, type: :job do it 'updates Cti::Logs from that number with valid "preferences" hash' do described_class.perform_now(phone) - expect(log_prefs).to all(include('from' => a_kind_of(Array))) + expect(log_prefs).not_to eq(Array.new(5) { nil }) end end end diff --git a/spec/models/cti/log_spec.rb b/spec/models/cti/log_spec.rb index e6ae8bfb3..a5a1bdd37 100644 --- a/spec/models/cti/log_spec.rb +++ b/spec/models/cti/log_spec.rb @@ -11,6 +11,18 @@ RSpec.describe Cti::Log do expect(described_class.log(user)).to match(hash_including(:list, :assets)) end + context 'when pretty is not generated' do + let(:log) { create(:'cti/log') } + + before do + log.update_column(:preferences, nil) + end + + it 'does fallback generate the pretty value' do + expect(log.reload.attributes['from_pretty']).to eq('+49 30 609854180') + end + end + context 'when over 60 Log records exist' do subject!(:cti_logs) do 61.times.map do |_i| # rubocop:disable Performance/TimesMap diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 10477a173..2377e07e8 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1296,8 +1296,8 @@ RSpec.describe User, type: :model do user.update(phone: '0123456789') Observer::Transaction.commit Scheduler.worker(true) - end.to change { logs.map(&:reload).map(&:preferences) } - .to(Array.new(5) { {} }) + end.to change { logs.map(&:reload).map { |log| log.preferences[:from] } } + .to(Array.new(5) { nil }) end end @@ -1307,8 +1307,8 @@ RSpec.describe User, type: :model do user.update(phone: '') Observer::Transaction.commit Scheduler.worker(true) - end.to change { logs.map(&:reload).map(&:preferences) } - .to(Array.new(5) { {} }) + end.to change { logs.map(&:reload).map { |log| log.preferences[:from] } } + .to(Array.new(5) { nil }) end end @@ -1318,8 +1318,8 @@ RSpec.describe User, type: :model do user.update(phone: nil) Observer::Transaction.commit Scheduler.worker(true) - end.to change { logs.map(&:reload).map(&:preferences) } - .to(Array.new(5) { {} }) + end.to change { logs.map(&:reload).map { |log| log.preferences[:from] } } + .to(Array.new(5) { nil }) end end end