Refactoring: Migrate fourth test case in cti_caller_id_test.rb

spec/factories/cti/log.rb

@@ -5,5 +5,9 @@ FactoryBot.define do
     from      { '4930609854180' }
     to        { '4930609811111' }
     call_id   { (Cti::Log.pluck(:call_id).map(&:to_i).max || 0).next } # has SQL UNIQUE constraint
+
+    trait :with_preferences do
+      preferences { Cti::CallerId.get_comment_preferences(from, 'from')&.last }
+    end
   end
 end

spec/models/ticket/article_spec.rb

@@ -8,17 +8,40 @@ RSpec.describe Ticket::Article, type: :model do
 
   describe 'Callbacks, Observers, & Async Transactions' do
     describe 'NULL byte handling (via ChecksAttributeValuesAndLength concern):' do
-      it 'removes them from subject on creation, if necessary (postgres doesn’t like them)' do
+      it 'removes them from #subject on creation, if necessary (postgres doesn’t like them)' do
         expect(create(:ticket_article, subject: "com test 1\u0000"))
           .to be_persisted
       end
 
-      it 'removes them from body on creation, if necessary (postgres doesn’t like them)' do
+      it 'removes them from #body on creation, if necessary (postgres doesn’t like them)' do
         expect(create(:ticket_article, body: "some\u0000message 123"))
           .to be_persisted
       end
     end
 
+    describe 'Cti::Log syncing:' do
+      context 'with existing Log records' do
+        context 'for an incoming call from an unknown number' do
+          let!(:log) { create(:'cti/log', :with_preferences, from: '491111222222', direction: 'in') }
+
+          context 'with that number in #body' do
+            subject(:article) { build(:ticket_article, body: <<~BODY) }
+              some message
+              +49 1111 222222
+            BODY
+
+            it 'does not modify any Log records (because CallerIds from article bodies have #level "maybe")' do
+              expect do
+                article.save
+                Observer::Transaction.commit
+                Scheduler.worker(true)
+              end.not_to change { log.reload.attributes }
+            end
+          end
+        end
+      end
+    end
+
     describe 'Auto-setting of outgoing Twitter article attributes (via bj jobs):' do
       subject!(:twitter_article) { create(:twitter_article, sender_name: 'Agent') }
       let(:channel) { Channel.find(twitter_article.ticket.preferences[:channel_id]) }

spec/models/ticket_spec.rb

@@ -291,6 +291,19 @@ RSpec.describe Ticket, type: :model do
       end
     end
 
+    describe 'Cti::CallerId syncing:' do
+      subject(:ticket) { build(:ticket) }
+      before { allow(Cti::CallerId).to receive(:build) }
+
+      it 'adds numbers in article bodies (via Cti::CallerId.build)' do
+        expect(Cti::CallerId).to receive(:build).with(ticket)
+
+        ticket.save
+        Observer::Transaction.commit
+        Scheduler.worker(true)
+      end
+    end
+
     describe 'Association & attachment management:' do
       it 'deletes all related ActivityStreams on destroy' do
         create_list(:activity_stream, 3, o: ticket)

spec/models/user_spec.rb

@@ -475,7 +475,7 @@ RSpec.describe User, type: :model do
     end
   end
 
-  describe 'Callbacks & Observers -' do
+  describe 'Callbacks, Observers, & Async Transactions -' do
     describe 'System-wide agent limit checks:' do
       let(:agent_role) { Role.lookup(name: 'Agent') }
       let(:admin_role) { Role.lookup(name: 'Admin') }
@@ -631,6 +631,140 @@ RSpec.describe User, type: :model do
         end
       end
     end
+    describe 'Cti::CallerId syncing:' do
+      context 'with a #phone attribute' do
+        subject(:user) { build(:user, phone: '1234567890') }
+
+        it 'adds CallerId record on creation (via Cti::CallerId.build)' do
+          expect(Cti::CallerId).to receive(:build).with(user)
+
+          user.save
+        end
+
+        it 'updates CallerId record on touch/update (via Cti::CallerId.build)' do
+          user.save
+
+          expect(Cti::CallerId).to receive(:build).with(user)
+
+          user.touch
+        end
+
+        it 'destroys CallerId record on deletion' do
+          user.save
+
+          expect { user.destroy }
+            .to change { Cti::CallerId.count }.by(-1)
+        end
+      end
+    end
+
+    describe 'Cti::Log syncing:' do
+      context 'with existing Log records' do
+        context 'for incoming calls from an unknown number' do
+          let!(:log) { create(:'cti/log', :with_preferences, from: '1234567890', direction: 'in') }
+
+          context 'when creating a new user with that number' do
+            subject(:user) { build(:user, phone: log.from) }
+
+            it 'populates #preferences[:from] hash in all associated Log records (in a bg job)' do
+              expect do
+                user.save
+                Observer::Transaction.commit
+                Scheduler.worker(true)
+              end.to change { log.reload.preferences[:from]&.first }
+                .to(hash_including('caller_id' => user.phone))
+            end
+          end
+
+          context 'when updating a user with that number' do
+            subject(:user) { create(:user) }
+
+            it 'populates #preferences[:from] hash in all associated Log records (in a bg job)' do
+              expect do
+                user.update(phone: log.from)
+                Observer::Transaction.commit
+                Scheduler.worker(true)
+              end.to change { log.reload.preferences[:from]&.first }
+                .to(hash_including('object' => 'User', 'o_id' => user.id))
+            end
+          end
+
+          context 'when creating a new user with an empty number' do
+            subject(:user) { build(:user, phone: '') }
+
+            it 'does not modify any Log records' do
+              expect do
+                user.save
+                Observer::Transaction.commit
+                Scheduler.worker(true)
+              end.not_to change { log.reload.attributes }
+            end
+          end
+
+          context 'when creating a new user with no number' do
+            subject(:user) { build(:user, phone: nil) }
+
+            it 'does not modify any Log records' do
+              expect do
+                user.save
+                Observer::Transaction.commit
+                Scheduler.worker(true)
+              end.not_to change { log.reload.attributes }
+            end
+          end
+        end
+
+        context 'for incoming calls from the given user' do
+          subject(:user) { create(:user, phone: '1234567890') }
+          let!(:logs) { create_list(:'cti/log', 5, :with_preferences, from: user.phone, direction: 'in') }
+
+          context 'when updating #phone attribute' do
+            context 'to another number' do
+              it 'empties #preferences[:from] hash in all associated Log records (in a bg job)' do
+                expect do
+                  user.update(phone: '0123456789')
+                  Observer::Transaction.commit
+                  Scheduler.worker(true)
+                end.to change { logs.map(&:reload).map(&:preferences) }
+                  .to(Array.new(5) { {} })
+              end
+            end
+
+            context 'to an empty string' do
+              it 'empties #preferences[:from] hash in all associated Log records (in a bg job)' do
+                expect do
+                  user.update(phone: '')
+                  Observer::Transaction.commit
+                  Scheduler.worker(true)
+                end.to change { logs.map(&:reload).map(&:preferences) }
+                  .to(Array.new(5) { {} })
+              end
+            end
+
+            context 'to nil' do
+              it 'empties #preferences[:from] hash in all associated Log records (in a bg job)' do
+                expect do
+                  user.update(phone: nil)
+                  Observer::Transaction.commit
+                  Scheduler.worker(true)
+                end.to change { logs.map(&:reload).map(&:preferences) }
+                  .to(Array.new(5) { {} })
+              end
+            end
+          end
+
+          context 'when updating attributes other than #phone' do
+            it 'does not modify any Log records' do
+              expect do
+                user.update(mobile: '2345678901')
+                Observer::Transaction.commit
+                Scheduler.worker(true)
+              end.not_to change { logs.map(&:reload).map(&:attributes) }
+            end
+          end
+        end
+      end
+    end
   end
 
 end

test/unit/cti_caller_id_test.rb

@@ -56,158 +56,6 @@ class CtiCallerIdTest < ActiveSupport::TestCase
     Scheduler.worker(true)
   end
 
-  test '4 touch caller log / don\'t touch caller log' do
-    5.times do |count|
-      travel 2.seconds
-      Cti::Log.process(
-        'cause'     => '',
-        'event'     => 'newCall',
-        'user'      => 'user 1',
-        'from'      => '491111222222',
-        'to'        => '4930600000000',
-        'callId'    => "touch-loop-#{count}",
-        'direction' => 'in',
-      )
-    end
-
-    # do not update Cti::Log on user touch
-    last_updated_at = Cti::Log.order(updated_at: :desc).first.updated_at
-    travel 10.minutes
-    @agent1.reload
-    @agent1.touch
-    Observer::Transaction.commit
-    Scheduler.worker(true)
-    assert_equal(last_updated_at, Cti::Log.order(updated_at: :desc).first.updated_at)
-
-    # do update old Cti::Log on phone update of user
-    @agent1.reload
-    @agent1.phone = '+49 1111 222222 999'
-    @agent1.save!
-    Observer::Transaction.commit
-    Scheduler.worker(true)
-    assert_not_equal(last_updated_at, Cti::Log.order(updated_at: :desc).first.updated_at)
-
-    # new call with not known number
-    travel 10.minutes
-    Cti::Log.process(
-      'cause'     => '',
-      'event'     => 'newCall',
-      'user'      => 'user 1',
-      'from'      => '49111122222277',
-      'to'        => '4930600000000',
-      'callId'    => 'touch-loop-20',
-      'direction' => 'in',
-    )
-
-    # set not known number for agent1
-    last_updated_at = Cti::Log.order(updated_at: :desc).first.updated_at
-    travel 10.minutes
-    @agent1.reload
-    @agent1.phone = '+49 1111 222222 77'
-    @agent1.save!
-    Observer::Transaction.commit
-    Scheduler.worker(true)
-    assert_not_equal(last_updated_at, Cti::Log.order(updated_at: :desc).first.updated_at)
-
-    # verify last updated entry
-    last = Cti::Log.order(updated_at: :desc).first
-    assert_equal('49111122222277', last.preferences[:from][0][:caller_id])
-    assert_nil(last.preferences[:from][0][:comment])
-    assert_equal('known', last.preferences[:from][0][:level])
-    assert_equal('User', last.preferences[:from][0][:object])
-    assert_equal(@agent1.id, last.preferences[:from][0][:o_id])
-
-    # create new user with no phone number
-    last_updated_at = Cti::Log.order(updated_at: :desc).first.updated_at
-    travel 30.minutes
-    agent4 = User.create!(
-      login:         'ticket-caller_id-agent4@example.com',
-      firstname:     'CallerId',
-      lastname:      'Agent4',
-      email:         'ticket-caller_id-agent4@example.com',
-      active:        true,
-      updated_by_id: 1,
-      created_by_id: 1,
-    )
-    Observer::Transaction.commit
-    Scheduler.worker(true)
-    assert_equal(last_updated_at, Cti::Log.order(updated_at: :desc).first.updated_at)
-
-    # verify if caller log is updated with '' value for phone
-    agent4.reload
-    agent4.phone = ''
-    agent4.save!
-    Observer::Transaction.commit
-    Scheduler.worker(true)
-    assert_equal(last_updated_at, Cti::Log.order(updated_at: :desc).first.updated_at)
-
-    # verify if caller log is updated with nil value for phone
-    agent4.reload
-    agent4.phone = nil
-    agent4.save!
-    Observer::Transaction.commit
-    Scheduler.worker(true)
-
-    # verify if caller log is updated with existing caller log value for phone
-    assert_equal(last_updated_at, Cti::Log.order(updated_at: :desc).first.updated_at)
-    agent4.reload
-    agent4.phone = '+49 1111 222222'
-    agent4.save!
-    Observer::Transaction.commit
-    Scheduler.worker(true)
-    assert_not_equal(last_updated_at, Cti::Log.order(updated_at: :desc).first.updated_at)
-
-    # verify if caller log is no value change for phone
-    last_updated_at = Cti::Log.order(updated_at: :desc).first.updated_at
-    travel 30.minutes
-    agent4.save!
-    Observer::Transaction.commit
-    Scheduler.worker(true)
-    assert_equal(last_updated_at, Cti::Log.order(updated_at: :desc).first.updated_at)
-
-    # verify if caller log is updated with '' value for phone
-    last_updated_at = Cti::Log.order(updated_at: :desc).first.updated_at
-    travel 30.minutes
-    agent4.reload
-    agent4.phone = ''
-    agent4.save!
-    Observer::Transaction.commit
-    Scheduler.worker(true)
-    assert_not_equal(last_updated_at, Cti::Log.order(updated_at: :desc).first.updated_at)
-
-    # verify if caller log is updated if new ticket with existing caller id is created
-    last_updated_at = Cti::Log.order(updated_at: :desc).first.updated_at
-    travel 30.minutes
-    last_caller_id_count = Cti::CallerId.count
-    ticket1 = Ticket.create!(
-      title:         'some caller id test 1',
-      group:         Group.lookup(name: 'Users'),
-      customer:      @customer1,
-      state:         Ticket::State.lookup(name: 'new'),
-      priority:      Ticket::Priority.lookup(name: '2 normal'),
-      updated_by_id: @customer1.id,
-      created_by_id: @customer1.id,
-    )
-    article1 = Ticket::Article.create!(
-      ticket_id:     ticket1.id,
-      from:          'some_sender@example.com',
-      to:            'some_recipient@example.com',
-      subject:       'some subject',
-      message_id:    'some@id',
-      body:          "some message\n+49 1111 222222",
-      internal:      false,
-      sender:        Ticket::Article::Sender.where(name: 'Customer').first,
-      type:          Ticket::Article::Type.where(name: 'email').first,
-      updated_by_id: @customer1.id,
-      created_by_id: @customer1.id,
-    )
-    Observer::Transaction.commit
-    Scheduler.worker(true)
-    assert_equal(last_caller_id_count + 2, Cti::CallerId.count)
-    assert_equal(last_updated_at, Cti::Log.order(updated_at: :desc).first.updated_at)
-
-  end
-
   test '5 probe if caller log need to be pushed' do
 
     Cti::Log.process(