Browse Source

Make new CallerId records adopt matching call logs (partially fixes #2057)

Ryan Lue 6 years ago
parent
commit
0afd2ba741

+ 13 - 0
app/jobs/update_cti_logs_by_caller_job.rb

@@ -0,0 +1,13 @@
+class UpdateCtiLogsByCallerJob < ApplicationJob
+  def perform(phone, limit: 60, offset: 0)
+    preferences = Cti::CallerId.get_comment_preferences(phone, 'from')&.last
+
+    Cti::Log.where(from: phone, direction: 'in')
+            .order(created_at: :desc)
+            .limit(limit)
+            .offset(offset)
+            .each do |log|
+      log.update(preferences: preferences)
+    end
+  end
+end

+ 13 - 0
app/models/cti/caller_id.rb

@@ -4,6 +4,11 @@ module Cti
 
     DEFAULT_COUNTRY_ID = '49'.freeze
 
+    # adopt/orphan matching Cti::Log records
+    # (see https://github.com/zammad/zammad/issues/2057)
+    after_commit :update_cti_logs, on: :destroy
+    after_commit :update_cti_logs_with_fg_optimization, on: :create
+
 =begin
 
   Cti::CallerId.maybe_add(
@@ -253,5 +258,13 @@ returns
       nil
     end
 
+    def update_cti_logs
+      UpdateCtiLogsByCallerJob.perform_later(caller_id)
+    end
+
+    def update_cti_logs_with_fg_optimization
+      UpdateCtiLogsByCallerJob.perform_now(caller_id, limit: 20)
+      UpdateCtiLogsByCallerJob.perform_later(caller_id, limit: 40, offset: 20)
+    end
   end
 end

+ 9 - 0
spec/factories/cti/caller_id.rb

@@ -0,0 +1,9 @@
+FactoryBot.define do
+  factory :cti_caller_id, class: 'cti/caller_id' do
+    caller_id '1234567890'
+    level     :known
+    object    :User
+    o_id      { User.last.id }
+    user_id   { User.last.id }
+  end
+end

+ 30 - 0
spec/jobs/update_cti_logs_by_caller_job_spec.rb

@@ -0,0 +1,30 @@
+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) }
+
+  it 'accepts a phone number' do
+    expect { described_class.perform_now(phone) }
+      .not_to raise_error
+  end
+
+  context 'with no user matching provided phone number' do
+    it 'updates Cti::Logs from that number with "preferences" => {}' do
+      described_class.perform_now(phone)
+
+      log_prefs.each { |p| expect(p).to be_empty }
+    end
+  end
+
+  context 'with existing user matching provided phone number' do
+    before { create(:user, phone: phone) }
+
+    it 'updates Cti::Logs from that number with valid "preferences" hash' do
+      described_class.perform_now(phone)
+
+      log_prefs.each { |p| expect(p).to include('from' => a_kind_of(Array)) }
+    end
+  end
+end

+ 32 - 0
spec/models/cti/caller_id_spec.rb

@@ -1,6 +1,8 @@
 require 'rails_helper'
 
 RSpec.describe Cti::CallerId do
+  subject { create(:cti_caller_id, caller_id: phone) }
+  let(:phone) { '1234567890' }
 
   describe 'extract_numbers' do
     it { expect(described_class.extract_numbers("some text\ntest 123")).to eq [] }
@@ -31,4 +33,34 @@ RSpec.describe Cti::CallerId do
     it { expect(described_class.normalize_number('0043 30 60 00 00 00-0')).to eq '4330600000000' }
     it { expect(described_class.normalize_number('1-888-407-4747')).to eq '18884074747' }
   end
+
+  context 'on creation' do
+    it 'adopts CTI Logs from same number (via UpdateCtiLogsByCallerJob)' do
+      allow(UpdateCtiLogsByCallerJob).to receive(:perform_later).with(any_args)
+
+      subject # create CallerId record
+
+      expect(UpdateCtiLogsByCallerJob).to have_received(:perform_later)
+    end
+
+    it 'splits job into fg and bg (for more responsive UI – see #2057)' do
+      allow(UpdateCtiLogsByCallerJob).to receive(:perform_now).with(any_args)
+      allow(UpdateCtiLogsByCallerJob).to receive(:perform_later).with(any_args)
+
+      subject # create CallerId record
+
+      expect(UpdateCtiLogsByCallerJob).to have_received(:perform_now).with(phone, limit: 20)
+      expect(UpdateCtiLogsByCallerJob).to have_received(:perform_later).with(phone, limit: 40, offset: 20)
+    end
+  end
+
+  context 'on destruction' do
+    before { subject }
+
+    it 'orphans CTI Logs from same number (via UpdateCtiLogsByCallerJob)' do
+      allow(UpdateCtiLogsByCallerJob).to receive(:perform_later).with(phone)
+      subject.destroy
+      expect(UpdateCtiLogsByCallerJob).to have_received(:perform_later)
+    end
+  end
 end