From d425b0fd19a1fdd11e5da0dfbe406f93444f9179 Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Tue, 5 Feb 2019 15:33:40 +0800 Subject: [PATCH] Refactoring: Migrate fourth test case in cti_caller_id_test.rb --- spec/factories/cti/log.rb | 4 + spec/models/ticket/article_spec.rb | 27 ++++- spec/models/ticket_spec.rb | 13 +++ spec/models/user_spec.rb | 136 +++++++++++++++++++++++++- test/unit/cti_caller_id_test.rb | 152 ----------------------------- 5 files changed, 177 insertions(+), 155 deletions(-) diff --git a/spec/factories/cti/log.rb b/spec/factories/cti/log.rb index 8118a6c62..b522963ad 100644 --- a/spec/factories/cti/log.rb +++ b/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 diff --git a/spec/models/ticket/article_spec.rb b/spec/models/ticket/article_spec.rb index 09c8c1263..0f4f335cc 100644 --- a/spec/models/ticket/article_spec.rb +++ b/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]) } diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index a712fce2d..b4f88156c 100644 --- a/spec/models/ticket_spec.rb +++ b/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) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 09be4b516..2aca5c1d0 100644 --- a/spec/models/user_spec.rb +++ b/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 diff --git a/test/unit/cti_caller_id_test.rb b/test/unit/cti_caller_id_test.rb index a5c6b5832..b79b6f7f3 100644 --- a/test/unit/cti_caller_id_test.rb +++ b/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(