From 6309eacc55f28ae4cb67f128d36085447537f3c1 Mon Sep 17 00:00:00 2001 From: Rolf Schmidt Date: Thu, 9 Jul 2020 16:43:36 +0200 Subject: [PATCH] Fixes #3110 - ServiceNow mails from other service providers are not detected. --- .../channel/filter/service_now_check.rb | 72 +++++++++++++------ ...9094556_issue_3110_service_now_provider.rb | 13 ++++ .../issue3110_service_now_provider_spec.rb | 27 +++++++ spec/factories/external_sync.rb | 4 ++ spec/models/channel/email_parser_spec.rb | 43 ++++++----- 5 files changed, 121 insertions(+), 38 deletions(-) create mode 100644 db/migrate/20200709094556_issue_3110_service_now_provider.rb create mode 100644 spec/db/migrate/issue3110_service_now_provider_spec.rb create mode 100644 spec/factories/external_sync.rb diff --git a/app/models/channel/filter/service_now_check.rb b/app/models/channel/filter/service_now_check.rb index 45f188d43..3f24d40ff 100644 --- a/app/models/channel/filter/service_now_check.rb +++ b/app/models/channel/filter/service_now_check.rb @@ -4,23 +4,25 @@ module Channel::Filter::ServiceNowCheck # This filter will run pre and post def self.run(_channel, mail, ticket = nil, _article = nil, _session_user = nil) - source_id = self.source_id(from: mail[:from], subject: mail[:subject]) + return if mail['x-servicenow-generated'].blank? + + source_id = self.source_id(subject: mail[:subject]) return if source_id.blank? + source_name = self.source_name(from: mail[:from]) + return if source_name.blank? + # check if we can followup by existing service now relation if ticket.blank? - sync_entry = ExternalSync.find_by( - source: 'ServiceNow', - source_id: source_id, - object: 'Ticket', + from_sync_entry( + mail: mail, + source_name: source_name, + source_id: source_id, ) - return if sync_entry.blank? - - mail[ 'x-zammad-ticket-id'.to_sym ] = sync_entry.o_id return end - ExternalSync.create_with(source_id: source_id).find_or_create_by(source: 'ServiceNow', object: 'Ticket', o_id: ticket.id) + ExternalSync.create_with(source_id: source_id).find_or_create_by(source: source_name, object: 'Ticket', o_id: ticket.id) end =begin @@ -28,7 +30,7 @@ module Channel::Filter::ServiceNowCheck This function returns the source id of the service now email if given. source_id = Channel::Filter::ServiceNowCheck.source_id( - from: 'test@servicnow.com', + from: 'test@service-now.com', subject: 'Incident INC12345 --- test', ) @@ -38,16 +40,7 @@ returns: =end - def self.source_id(from: '', subject: '') - - # check if data is sent by service now - begin - return if Mail::AddressList.new(from).addresses.none? do |line| - line.address.end_with?('@service-now.com') - end - rescue - Rails.logger.info "Unable to parse email address in '#{from}'" - end + def self.source_id(subject: '') # check if we can find the service now relation source_id = nil @@ -57,4 +50,43 @@ returns: source_id end + +=begin + +This function returns the sync id of the service now email if given. + + source_name = Channel::Filter::ServiceNowCheck.source_name( + from: 'test@service-now.com', + ) + +returns: + + source_name = 'ServiceNow-test@service-now.com' + +=end + + def self.source_name(from:) + result = nil + begin + Mail::AddressList.new(from).addresses.each do |line| + result = "ServiceNow-#{line.address}" + break + end + rescue + Rails.logger.info "Unable to parse email address in '#{from}'" + end + + result + end + + def self.from_sync_entry(mail:, source_name:, source_id:) + sync_entry = ExternalSync.find_by( + source: source_name, + source_id: source_id, + object: 'Ticket', + ) + return if sync_entry.blank? + + mail[ 'x-zammad-ticket-id'.to_sym ] = sync_entry.o_id + end end diff --git a/db/migrate/20200709094556_issue_3110_service_now_provider.rb b/db/migrate/20200709094556_issue_3110_service_now_provider.rb new file mode 100644 index 000000000..cf52b3066 --- /dev/null +++ b/db/migrate/20200709094556_issue_3110_service_now_provider.rb @@ -0,0 +1,13 @@ +class Issue3110ServiceNowProvider < ActiveRecord::Migration[5.2] + def change + + # return if it's a new setup + return if !Setting.find_by(name: 'system_init_done') + + ExternalSync.where(source: 'ServiceNow').find_each do |row| + article = Ticket.find(row.o_id).articles.first + source_name = Channel::Filter::ServiceNowCheck.source_name(from: article.from) + row.update(source: source_name) + end + end +end diff --git a/spec/db/migrate/issue3110_service_now_provider_spec.rb b/spec/db/migrate/issue3110_service_now_provider_spec.rb new file mode 100644 index 000000000..57b64e98f --- /dev/null +++ b/spec/db/migrate/issue3110_service_now_provider_spec.rb @@ -0,0 +1,27 @@ +require 'rails_helper' + +RSpec.describe Issue3110ServiceNowProvider, type: :db_migration do + + let(:ticket) { create(:ticket) } + let(:external_sync) do + create(:external_sync, + source: 'ServiceNow', + source_id: 'INC678439', + object: 'Ticket', + o_id: ticket.id) + end + + before do + create(:ticket_article, + ticket: ticket, + subject: 'Incident INC678439 -- zugewiesen an EXT-XXXINIS', + from: 'zam@mad-service-now.com') + end + + it 'does migrates obsolete ServiceNow ExternalSync references' do + expect { migrate } + .to change { external_sync.reload.source } + .from('ServiceNow') + .to('ServiceNow-zam@mad-service-now.com') + end +end diff --git a/spec/factories/external_sync.rb b/spec/factories/external_sync.rb new file mode 100644 index 000000000..419a9e37f --- /dev/null +++ b/spec/factories/external_sync.rb @@ -0,0 +1,4 @@ +FactoryBot.define do + factory :external_sync do + end +end diff --git a/spec/models/channel/email_parser_spec.rb b/spec/models/channel/email_parser_spec.rb index 2fb0efb4c..9475ac498 100644 --- a/spec/models/channel/email_parser_spec.rb +++ b/spec/models/channel/email_parser_spec.rb @@ -993,29 +993,36 @@ RSpec.describe Channel::EmailParser, type: :model do end describe 'ServiceNow handling' do - context 'when emails with service now reference are sent' do + + context 'new Ticket' do let(:mail_file) { Rails.root.join('test/data/mail/mail089.box') } - let(:mail_file_answer) { Rails.root.join('test/data/mail/mail090.box') } - let(:raw_mail_answer) { File.read(mail_file_answer) } - it 'does create a ticket with external sync reference' do - expect { described_class.new.process({}, raw_mail) } - .to change(Ticket, :count).by(1) - .and change(Ticket::Article, :count).by(1) - .and change(ExternalSync, :count).by(1) + it 'creates an ExternalSync reference' do + described_class.new.process({}, raw_mail) - expect(ExternalSync.last.source).to eq('ServiceNow') - expect(ExternalSync.last.source_id).to eq('INC678439') - expect(ExternalSync.last.object).to eq('Ticket') - expect(ExternalSync.last.o_id).to eq(Ticket.last.id) - expect(Ticket.last.articles.last.subject).to eq('Incident INC678439 -- zugewiesen an EXT-XXXINIS') + expect(ExternalSync.last).to have_attributes( + source: 'ServiceNow-example@service-now.com', + source_id: 'INC678439', + object: 'Ticket', + o_id: Ticket.last.id, + ) + end + end - expect { described_class.new.process({}, raw_mail_answer) } - .to change(Ticket, :count).by(0) - .and change(Ticket::Article, :count).by(1) - .and change(ExternalSync, :count).by(0) + context 'follow up' do - expect(Ticket.last.articles.last.subject).to eq('Incident INC678439 -- Arbeitsnotizen beigefügt') + let(:mail_file) { Rails.root.join('test/data/mail/mail090.box') } + let(:ticket) { create(:ticket) } + let!(:external_sync) do + create(:external_sync, + source: 'ServiceNow-example@service-now.com', + source_id: 'INC678439', + object: 'Ticket', + o_id: ticket.id,) + end + + it 'adds Article to existing Ticket' do + expect { described_class.new.process({}, raw_mail) }.to change { ticket.reload.articles.count } end end end