Fixes zammad#3550 - CTI Log performance bad because of reformatting o…

…f the phone number.

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(->

app/assets/javascripts/app/views/integration/cti.jst.eco

@@ -76,19 +76,22 @@
     </table>
   </div>
 
-  <p><%- @T('Default caller id.') %>
+  <p><%- @T('Settings') %>
 
   <div class="settings-entry">
     <table class="settings-list" style="width: 100%;">
       <thead>
         <tr>
-          <th width="50%"><%- @T('Default caller id') %>
-          <th width="50%"><%- @T('Note') %>
+          <th width="50%"><%- @T('Name') %>
+          <th width="50%"><%- @T('Description') %>
       </thead>
       <tbody>
         <tr>
           <td class="settings-list-control-cell"><input name="default_caller_id" value="<%= @config.outbound.default_caller_id %>" placeholder="4930609854189" class="form-control form-control--small js-summary">
           <td class="settings-list-row-control"><%- @T('Default caller id for outbound calls.') %>
+        <tr>
+          <td class="settings-list-control-cell js-viewLimit">
+          <td class="settings-list-row-control"><%- @T('Shown records in caller log.') %>
       </tbody>
     </table>
   </div>
@@ -121,4 +124,4 @@
   </div>
 
   <button type="submit" class="btn btn--primary js-submit"><%- @T('Save') %></button>
-</form>
+</form>

app/models/cti/log.rb

@@ -4,10 +4,12 @@ class Log < ApplicationModel
 
     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 @@ def self.log(current_user)
     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 @@ def self.process(params)
     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 @@ def self.cleanup(diff = 12.months)
     # 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 @@ def attributes
       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 @@ def best_customer_id_of_log_entry
       customer_id
     end
 
+    def self.view_limit
+      Hash(Setting.get('cti_config'))['view_limit'] || 60
+    end
   end
 end

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

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

spec/jobs/update_cti_logs_by_caller_job_spec.rb

@@ -3,7 +3,7 @@
 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 @@
     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 @@
     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

spec/models/cti/log_spec.rb

@@ -11,6 +11,18 @@
       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

spec/models/user_spec.rb

@@ -1296,8 +1296,8 @@
                   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 @@
                   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 @@
                   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