Merge OTRS bugfixing branch into develop.

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

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

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

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)

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']),

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<Hash{String => 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]  || '',
+          Limit:     opts[:limit]  || '',
-          Offset:    args[:offset] || '',
+          Offset:    opts[:offset] || '',
-          Diff:      args[:diff] ? 1 : 0
+          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!' if !result['Success']
-        raise 'API key not valid!'
+        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)
-
+        raise 'Invalid response' if !result
-        return result if result
+        result
-
-        raise 'Invalid response'
       end
 
       def handle_response(response)

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
 

lib/import/zendesk.rb

@@ -62,6 +62,7 @@ module Import::Zendesk
 
   def checks
     check_import_mode
+    check_system_init_done
     connection_test
   end
 end

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\" <kunde2@kunde.de>,"
+}

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"
+}

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
+  context 'import mode' do
-    expect(Setting).to receive(:get).with('import_mode').and_return(true)
+
-    expect( described_class.check_import_mode ).to be true
+    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
+  context 'system init' do
-    expect(Setting).to receive(:get).with('import_mode').and_return(false)
+
-    expect { described_class.check_import_mode }.to raise_error(RuntimeError)
+    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

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

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" <kunde2@kunde.de>,',
+        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

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
+  context '.load' do
-    expect(described_class).to respond_to('list')
-  end
 
-  it 'responds to connection_test' do
+    it 'responds to load' do
-    expect(described_class).to respond_to('connection_test')
+      expect(described_class).to respond_to(:load)
-  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')
     end
 
-    it 'is not active if args are given' do
+    context 'caching request results' do
-      expect(UserAgent).to receive(:post).twice.and_return(response)
+
-      described_class.load('Ticket', offset: 10)
+      let(:response) {
-      described_class.load('Ticket', offset: 20)
+        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

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

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

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