diff --git a/app/models/user.rb b/app/models/user.rb index cd6aa9deb..70f772f2c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -980,6 +980,7 @@ raise 'Minimum one user need to have admin permissions' def ensure_password return if password_empty? + return if Setting.get('import_mode') return if PasswordHash.crypted?(password) self.password = PasswordHash.crypt(password) end diff --git a/lib/import/helper.rb b/lib/import/helper.rb index 696f1c279..596508809 100644 --- a/lib/import/helper.rb +++ b/lib/import/helper.rb @@ -9,13 +9,16 @@ module Import raise 'System is not in import mode!' end - # log + def check_system_init_done + return true if !Setting.get('system_init_done') + raise 'System is already system_init_done!' + end + def log(message) thread_no = Thread.current[:thread_no] || '-' Rails.logger.info "thread##{thread_no}: #{message}" end - # utf8 convert def utf8_encode(data) data.each { |key, value| next if !value diff --git a/lib/import/otrs.rb b/lib/import/otrs.rb index b471eff30..9741a5de6 100644 --- a/lib/import/otrs.rb +++ b/lib/import/otrs.rb @@ -57,6 +57,7 @@ module Import def checks check_import_mode + check_system_init_done connection_test end @@ -82,6 +83,11 @@ module Import threads[thread] = Thread.new { + # In some environments the Model.reset_column_information + # is not reflected to threads. So an import error message appears. + # Reset needed model column information for each thread. + reset_database_information + Thread.current[:thread_no] = thread Thread.current[:loop_count] = 0 @@ -162,5 +168,9 @@ module Import def customer_user limit_import('CustomerUser', limit: 50) end + + def reset_database_information + ::Ticket.reset_column_information + end end end diff --git a/lib/import/otrs/article.rb b/lib/import/otrs/article.rb index b478811ff..a50c7669c 100644 --- a/lib/import/otrs/article.rb +++ b/lib/import/otrs/article.rb @@ -70,6 +70,8 @@ module Import def map(article) mapped = map_default(article) map_content_type(mapped) + mapped[:body] ||= '' + mapped end def map_default(article) diff --git a/lib/import/otrs/article_customer.rb b/lib/import/otrs/article_customer.rb index acb1e2d74..1ee83c56a 100644 --- a/lib/import/otrs/article_customer.rb +++ b/lib/import/otrs/article_customer.rb @@ -12,12 +12,22 @@ module Import class << self def find(article) - email = extract_email(article['From']) + email = local_email(article['From']) + return if !email user = ::User.find_by(email: email) user ||= ::User.find_by(login: email) user end + def local_email(from) + # TODO: should get unified with User#check_email + email = extract_email(from) + return if !email + email.downcase + end + + private + def extract_email(from) Mail::Address.new(from).address rescue @@ -38,7 +48,7 @@ module Import end def create(article) - email = self.class.extract_email(article['From']) + email = self.class.local_email(article['From']) ::User.create( login: email, firstname: extract_display_name(article['From']), diff --git a/lib/import/otrs/requester.rb b/lib/import/otrs/requester.rb index 69b1d4fb2..c3dbbf049 100644 --- a/lib/import/otrs/requester.rb +++ b/lib/import/otrs/requester.rb @@ -1,56 +1,96 @@ module Import module OTRS + + # @!attribute [rw] retry_sleep + # @return [Number] the sleep time between the request retries module Requester extend Import::Helper # rubocop:disable Style/ModuleFunction extend self - def load(object, args = {}) + attr_accessor :retry_sleep + + # Loads entries of the given object. + # + # @param object [String] the name of OTRS object + # @param [Hash] opts the options to load entries. + # @option opts [String] :limit the maximum amount of entries that should get loaded + # @option opts [String] :offset the offset where the entry listing should start + # @option opts [Boolean] :diff request only changed/added entries since the last import + # + # @example + # Import::OTRS::Requester.load('State', offset: '0', limit: '50') + # #=> [{'Name':'pending reminder', ...}, ...] + # + # @return [Array String, Number, nil, Hash, Array}>] + def load(object, opts = {}) @cache ||= {} - if args.empty? && @cache[object] + if opts.empty? && @cache[object] return @cache[object] end result = request_result( Subaction: 'Export', Object: object, - Limit: args[:limit] || '', - Offset: args[:offset] || '', - Diff: args[:diff] ? 1 : 0 + Limit: opts[:limit] || '', + Offset: opts[:offset] || '', + Diff: opts[:diff] ? 1 : 0 ) - return result if !args.empty? + return result if !opts.empty? @cache[object] = result @cache[object] end + # Lists the OTRS objects and their amount of importable entries. + # + # @example + # Import::OTRS::Requester.list #=> {'DynamicFields' => 5, ...} + # + # @return [Hash{String => Number}] key = OTRS object, value = amount def list request_result(Subaction: 'List') end - # TODO: refactor to something like .connected? + # Checks if the connection to the OTRS export endpoint works. + # + # @todo Refactor to something like .connected? + # + # @example + # Import::OTRS::Requester.connection_test #=> true + # + # @raise [RuntimeError] if the API key is not valid + # + # @return [true] always returns true def connection_test result = request_json({}) - return true if result['Success'] - raise 'API key not valid!' + raise 'API key not valid!' if !result['Success'] + true end private def request_result(params) + tries ||= 1 response = request_json(params) response['Result'] + rescue + # stop after 3 tries + raise if tries == 3 + + # try again + tries += 1 + sleep tries * (retry_sleep || 15) + retry end def request_json(params) response = post(params) result = handle_response(response) - - return result if result - - raise 'Invalid response' + raise 'Invalid response' if !result + result end def handle_response(response) diff --git a/lib/import/otrs/user.rb b/lib/import/otrs/user.rb index f32d05188..384d43f1f 100644 --- a/lib/import/otrs/user.rb +++ b/lib/import/otrs/user.rb @@ -38,7 +38,7 @@ module Import return false if !@local_user # only update roles if different (reduce sql statements) - if @local_user.role_ids == user[:role_ids] + if user[:role_ids] && user[:role_ids].sort == @local_user.role_ids.sort user.delete(:role_ids) end diff --git a/lib/import/zendesk.rb b/lib/import/zendesk.rb index 7d0d4c8ae..110124103 100644 --- a/lib/import/zendesk.rb +++ b/lib/import/zendesk.rb @@ -62,6 +62,7 @@ module Import::Zendesk def checks check_import_mode + check_system_init_done connection_test end end diff --git a/spec/fixtures/import/otrs/article/customer_phone_no_body.json b/spec/fixtures/import/otrs/article/customer_phone_no_body.json new file mode 100644 index 000000000..3348aa70a --- /dev/null +++ b/spec/fixtures/import/otrs/article/customer_phone_no_body.json @@ -0,0 +1,97 @@ +{ + "Age": 63188310, + "PriorityID": "3", + "ContentType": "text/plain; charset=utf-8", + "AttachmentIDOfHTMLBody": "1", + "DynamicField_SugarCRMCompanySelection": null, + "ServiceID": null, + "TicketFreeText11": null, + "DynamicField_ITSMDueDate": "2014-11-24 00:15:00", + "DynamicField_Topic": null, + "StateID": "2", + "DynamicField_Hostname": null, + "Body": null, + "DynamicField_ZammadMigratorChanged": null, + "EscalationTime": "0", + "Changed": "2014-11-21 00:21:08", + "OwnerID": "3", + "DynamicField_ZarafaTN": null, + "DynamicField_ProcessManagementActivityID": null, + "DynamicField_TopicID": null, + "DynamicField_ScomHostname": null, + "Owner": "agent-2", + "AgeTimeUnix": 63188309, + "TicketFreeKey11": null, + "ArticleID": "3970", + "Created": "2014-11-21 00:17:41", + "DynamicField_ScomUUID": null, + "DynamicField_TicketFreeText11": null, + "DynamicField_TicketFreeKey11": null, + "DynamicField_ITSMReviewRequired": "No", + "DynamicField_OpenExchangeTicketNumber": null, + "DynamicField_ITSMDecisionDate": null, + "ArticleTypeID": "5", + "QueueID": "1", + "ReplyTo": "", + "DynamicField_ITSMImpact": null, + "TicketID": "730", + "DynamicField_ITSMRecoveryStartTime": null, + "Cc": "", + "EscalationResponseTime": "0", + "DynamicField_ProcessManagementProcessID": null, + "IncomingTime": "1416525461", + "Charset": "utf-8", + "DynamicField_CheckboxExample": null, + "DynamicField_Location": null, + "CustomerUserID": "BetreuterKunde2", + "DynamicField_Vertriebsweg": null, + "Attachments": [], + "DynamicField_CustomerLocation": null, + "DynamicField_SugarCRMRemoteID": null, + "DynamicField_OpenExchangeTN": null, + "Service": "", + "Type": "Incident", + "ContentCharset": "utf-8", + "DynamicField_TETest": null, + "Responsible": "root@localhost", + "SenderType": "customer", + "ResponsibleID": "1", + "SLA": "", + "MimeType": "text/plain", + "DynamicField_Combine": null, + "Subject": "test #3", + "InReplyTo": "", + "RealTillTimeNotUsed": "0", + "DynamicField_ScomService": null, + "CustomerID": "3333333333", + "TypeID": "1", + "MessageID": "", + "Priority": "3 normal", + "To": "Postmaster", + "DynamicField_SugarCRMCompanySelectedID": null, + "UntilTime": 0, + "EscalationUpdateTime": "0", + "CreatedBy": "3", + "Queue": "Postmaster", + "DynamicField_ITSMRepairStartTime": null, + "ToRealname": "Postmaster", + "State": "closed successful", + "SenderTypeID": "3", + "DynamicField_ZammadMigratorChangedOld": "1", + "Title": "test #3", + "DynamicField_ScomState": null, + "References": "", + "DynamicField_Department": null, + "ArticleType": "phone", + "StateType": "closed", + "FromRealname": "Betreuter Kunde", + "EscalationSolutionTime": "0", + "LockID": "1", + "TicketNumber": "20141121305000012", + "DynamicField_ITSMDecisionResult": null, + "Lock": "unlock", + "CreateTimeUnix": "1416525460", + "SLAID": null, + "DynamicField_ITSMCriticality": null, + "From": "\"Betreuter Kunde\" ," +} \ No newline at end of file diff --git a/spec/fixtures/import/otrs/article/from_capital_case.json b/spec/fixtures/import/otrs/article/from_capital_case.json new file mode 100644 index 000000000..244ba2624 --- /dev/null +++ b/spec/fixtures/import/otrs/article/from_capital_case.json @@ -0,0 +1,97 @@ +{ + "Age": 63188310, + "PriorityID": "3", + "ContentType": "text/plain; charset=utf-8", + "AttachmentIDOfHTMLBody": "1", + "DynamicField_SugarCRMCompanySelection": null, + "ServiceID": null, + "TicketFreeText11": null, + "DynamicField_ITSMDueDate": "2014-11-24 00:15:00", + "DynamicField_Topic": null, + "StateID": "2", + "DynamicField_Hostname": null, + "Body": "test #3", + "DynamicField_ZammadMigratorChanged": null, + "EscalationTime": "0", + "Changed": "2014-11-21 00:21:08", + "OwnerID": "3", + "DynamicField_ZarafaTN": null, + "DynamicField_ProcessManagementActivityID": null, + "DynamicField_TopicID": null, + "DynamicField_ScomHostname": null, + "Owner": "agent-2", + "AgeTimeUnix": 63188309, + "TicketFreeKey11": null, + "ArticleID": "3970", + "Created": "2014-11-21 00:17:41", + "DynamicField_ScomUUID": null, + "DynamicField_TicketFreeText11": null, + "DynamicField_TicketFreeKey11": null, + "DynamicField_ITSMReviewRequired": "No", + "DynamicField_OpenExchangeTicketNumber": null, + "DynamicField_ITSMDecisionDate": null, + "ArticleTypeID": "5", + "QueueID": "1", + "ReplyTo": "", + "DynamicField_ITSMImpact": null, + "TicketID": "730", + "DynamicField_ITSMRecoveryStartTime": null, + "Cc": "", + "EscalationResponseTime": "0", + "DynamicField_ProcessManagementProcessID": null, + "IncomingTime": "1416525461", + "Charset": "utf-8", + "DynamicField_CheckboxExample": null, + "DynamicField_Location": null, + "CustomerUserID": "BetreuterKunde2", + "DynamicField_Vertriebsweg": null, + "Attachments": [], + "DynamicField_CustomerLocation": null, + "DynamicField_SugarCRMRemoteID": null, + "DynamicField_OpenExchangeTN": null, + "Service": "", + "Type": "Incident", + "ContentCharset": "utf-8", + "DynamicField_TETest": null, + "Responsible": "root@localhost", + "SenderType": "customer", + "ResponsibleID": "1", + "SLA": "", + "MimeType": "text/plain", + "DynamicField_Combine": null, + "Subject": "test #3", + "InReplyTo": "", + "RealTillTimeNotUsed": "0", + "DynamicField_ScomService": null, + "CustomerID": "3333333333", + "TypeID": "1", + "MessageID": "", + "Priority": "3 normal", + "To": "Postmaster", + "DynamicField_SugarCRMCompanySelectedID": null, + "UntilTime": 0, + "EscalationUpdateTime": "0", + "CreatedBy": "3", + "Queue": "Postmaster", + "DynamicField_ITSMRepairStartTime": null, + "ToRealname": "Postmaster", + "State": "closed successful", + "SenderTypeID": "3", + "DynamicField_ZammadMigratorChangedOld": "1", + "Title": "test #3", + "DynamicField_ScomState": null, + "References": "", + "DynamicField_Department": null, + "ArticleType": "phone", + "StateType": "closed", + "FromRealname": "Betreuter Kunde", + "EscalationSolutionTime": "0", + "LockID": "1", + "TicketNumber": "20141121305000012", + "DynamicField_ITSMDecisionResult": null, + "Lock": "unlock", + "CreateTimeUnix": "1416525460", + "SLAID": null, + "DynamicField_ITSMCriticality": null, + "From": "User@example.com" +} \ No newline at end of file diff --git a/spec/lib/import/helper_spec.rb b/spec/lib/import/helper_spec.rb index 78ea79db8..1ac356672 100644 --- a/spec/lib/import/helper_spec.rb +++ b/spec/lib/import/helper_spec.rb @@ -4,13 +4,30 @@ require 'lib/import/helper_examples' RSpec.describe Import::Helper do it_behaves_like 'Import::Helper' - it 'checks if import_mode is active' do - expect(Setting).to receive(:get).with('import_mode').and_return(true) - expect( described_class.check_import_mode ).to be true + context 'import mode' do + + it 'checks if import_mode is active' do + expect(Setting).to receive(:get).with('import_mode').and_return(true) + expect( described_class.check_import_mode ).to be true + end + + it 'throws an exception if import_mode is disabled' do + expect(Setting).to receive(:get).with('import_mode').and_return(false) + expect { described_class.check_import_mode }.to raise_error(RuntimeError) + end end - it 'throws an exception if import_mode is disabled' do - expect(Setting).to receive(:get).with('import_mode').and_return(false) - expect { described_class.check_import_mode }.to raise_error(RuntimeError) + context 'system init' do + + it 'checks if system_init_done is active' do + expect(Setting).to receive(:get).with('system_init_done').and_return(false) + expect( described_class.check_system_init_done ).to be true + end + + it 'throws an exception if system_init_done is disabled' do + expect(Setting).to receive(:get).with('system_init_done').and_return(true) + expect { described_class.check_system_init_done }.to raise_error(RuntimeError) + end end + end diff --git a/spec/lib/import/otrs/article_customer_sepc.rb b/spec/lib/import/otrs/article_customer_sepc.rb index 7afece12c..0645271b0 100644 --- a/spec/lib/import/otrs/article_customer_sepc.rb +++ b/spec/lib/import/otrs/article_customer_sepc.rb @@ -36,8 +36,34 @@ RSpec.describe Import::OTRS::ArticleCustomer do expect(User.last.login).to eq('user.hernandez@example.com') end - it 'creates customers with special from email sytax' do + it 'creates customers with special from email syntax' do expect { described_class.new(load_article_json('from_bracket_email_syntax')) }.to change { User.count }.by(1) expect(User.last.login).to eq('user@example.com') end + + it 'converts emails to downcase' do + Setting.set('import_mode', true) + expect { described_class.new(load_article_json('from_capital_case')) }.to change { User.count }.by(1) + expect(User.last.email).to eq('user@example.com') + expect(User.last.login).to eq('user@example.com') + end + + context '.find' do + + it 'returns nil if no email could be found' do + expect(described_class.find({})).to be nil + end + end + + context '.local_email' do + + it 'returns nil if no email could be found' do + expect(described_class.local_email(nil)).to be nil + end + + it 'returns the parameter if no email could be found' do + not_an_email = 'thisisnotanemail' + expect(described_class.local_email(not_an_email)).to eq(not_an_email) + end + end end diff --git a/spec/lib/import/otrs/article_spec.rb b/spec/lib/import/otrs/article_spec.rb index 1d2cb7227..7ad1d137d 100644 --- a/spec/lib/import/otrs/article_spec.rb +++ b/spec/lib/import/otrs/article_spec.rb @@ -131,4 +131,39 @@ RSpec.describe Import::OTRS::Article do updates_with(zammad_structure) end end + + context 'no article body' do + + let(:object_structure) { load_article_json('customer_phone_no_body') } + let(:zammad_structure) { + { + created_by_id: '3', + updated_by_id: 1, + ticket_id: '730', + id: '3970', + body: '', + from: '"Betreuter Kunde" ,', + to: 'Postmaster', + cc: '', + content_type: 'text/plain', + subject: 'test #3', + in_reply_to: '', + message_id: '', + references: '', + updated_at: '2014-11-21 00:21:08', + created_at: '2014-11-21 00:17:41', + type_id: 5, + internal: false, + sender_id: 2 + } + } + + it 'creates' do + creates_with(zammad_structure) + end + + it 'updates' do + updates_with(zammad_structure) + end + end end diff --git a/spec/lib/import/otrs/requester_spec.rb b/spec/lib/import/otrs/requester_spec.rb index f7b77cc4a..630a58a43 100644 --- a/spec/lib/import/otrs/requester_spec.rb +++ b/spec/lib/import/otrs/requester_spec.rb @@ -1,39 +1,54 @@ require 'rails_helper' RSpec.describe Import::OTRS::Requester do - it 'responds to load' do - expect(described_class).to respond_to('load') + + context '.list' do + it 'responds to list' do + expect(described_class).to respond_to(:list) + end end - it 'responds to list' do - expect(described_class).to respond_to('list') - end + context '.load' do - it 'responds to connection_test' do - expect(described_class).to respond_to('connection_test') - end - - context 'caching request results' do - - let(:response) { - response = double() - response_body = double() - expect(response_body).to receive(:to_s).at_least(:once).and_return('{"Result": {}}') - expect(response).to receive('success?').at_least(:once).and_return(true) - expect(response).to receive('body').at_least(:once).and_return(response_body) - response - } - - it 'is active if no args are given' do - expect(UserAgent).to receive(:post).and_return(response) - described_class.load('Ticket') - described_class.load('Ticket') + it 'responds to load' do + expect(described_class).to respond_to(:load) end - it 'is not active if args are given' do - expect(UserAgent).to receive(:post).twice.and_return(response) - described_class.load('Ticket', offset: 10) - described_class.load('Ticket', offset: 20) + context 'caching request results' do + + let(:response) { + response = double() + response_body = double() + expect(response_body).to receive(:to_s).at_least(:once).and_return('{"Result": {}}') + expect(response).to receive('success?').at_least(:once).and_return(true) + expect(response).to receive('body').at_least(:once).and_return(response_body) + response + } + + it 'is active if no args are given' do + expect(UserAgent).to receive(:post).and_return(response) + described_class.load('Ticket') + described_class.load('Ticket') + end + + it 'is not active if args are given' do + expect(UserAgent).to receive(:post).twice.and_return(response) + described_class.load('Ticket', offset: 10) + described_class.load('Ticket', offset: 20) + end end end + + context '.connection_test' do + it 'responds to connection_test' do + expect(described_class).to respond_to(:connection_test) + end + end + + it 'retries request 3 times on errors' do + expect(UserAgent).to receive(:post).and_raise(Errno::ECONNRESET).exactly(3).times + # disable sleep time to speed up tests + described_class.retry_sleep = 0 + expect { described_class.list }.to raise_error(Errno::ECONNRESET) + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3625352d4..7c932eebb 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -104,7 +104,7 @@ RSpec.describe User do end end - context '.by_reset_token' do + context '#by_reset_token' do it 'returns a User instance for existing tokens' do token = create(:token_password_reset) @@ -133,4 +133,20 @@ RSpec.describe User do end end + context 'import' do + + it "doesn't change imported passwords" do + + # mock settings calls + expect(Setting).to receive(:get).with('import_mode').and_return(true) + allow(Setting).to receive(:get) + + user = build(:user, password: '{sha2}dd9c764fa7ea18cd992c8600006d3dc3ac983d1ba22e9ba2d71f6207456be0ba') # zammad + expect { + user.save + }.to_not change { + user.password + } + end + end end diff --git a/test/integration/otrs_import_test.rb b/test/integration/otrs_import_test.rb index 47d078278..307a949ae 100644 --- a/test/integration/otrs_import_test.rb +++ b/test/integration/otrs_import_test.rb @@ -13,6 +13,7 @@ class OtrsImportTest < ActiveSupport::TestCase Setting.set('import_otrs_endpoint', ENV['IMPORT_OTRS_ENDPOINT']) Setting.set('import_otrs_endpoint_key', ENV['IMPORT_OTRS_ENDPOINT_KEY']) Setting.set('import_mode', true) + Setting.set('system_init_done', false) Import::OTRS.start # check settings items diff --git a/test/integration/zendesk_import_test.rb b/test/integration/zendesk_import_test.rb index dcff85b07..709ff1f2f 100644 --- a/test/integration/zendesk_import_test.rb +++ b/test/integration/zendesk_import_test.rb @@ -17,6 +17,7 @@ class ZendeskImportTest < ActiveSupport::TestCase Setting.set('import_zendesk_endpoint_key', ENV['IMPORT_ZENDESK_ENDPOINT_KEY']) Setting.set('import_zendesk_endpoint_username', ENV['IMPORT_ZENDESK_ENDPOINT_USERNAME']) Setting.set('import_mode', true) + Setting.set('system_init_done', false) Import::Zendesk.start # check statistic count