Fixed issue #520 - OTRS import: Customer user is not overtaken from first article and into created_by_id.

lib/import/otrs/article_customer.rb

@@ -4,44 +4,44 @@ module Import
       include Import::Helper
 
       def initialize(article)
-        user = import(article)
+        import(article)
-        return if !user
-        article['created_by_id'] = user.id
       rescue Exceptions::UnprocessableEntity => e
         log "ERROR: Can't extract customer from Article #{article[:id]}"
       end
 
+      class << self
+
+        def find(article)
+          email = extract_email(article['From'])
+          user   = ::User.find_by(email: email)
+          user ||= ::User.find_by(login: email)
+          user
+        end
+
+        def extract_email(from)
+          Mail::Address.new(from).address
+        rescue
+          return from if from !~ /<\s*([^\s]+)/
+          $1
+        end
+      end
+
       private
 
       def import(article)
-        find_user_or_create(article)
+        find_or_create(article)
       end
 
-      def extract_email(from)
+      def find_or_create(article)
-        Mail::Address.new(from).address
+        return if self.class.find(article)
-      rescue
+        create(article)
-        return from if from !~ /<\s*([^\s]+)/
-        $1
       end
 
-      def find_user_or_create(article)
+      def create(article)
-        user = user_found?(article)
+        email = self.class.extract_email(article['From'])
-        return user if user
-        create_user(article)
-      end
-
-      def user_found?(article)
-        email = extract_email(article['From'])
-        user   = ::User.find_by(email: email)
-        user ||= ::User.find_by(login: email)
-        user
-      end
-
-      def create_user(article)
-        email = extract_email(article['From'])
         ::User.create(
           login:         email,
-          firstname:     extract_display_name(article['from']),
+          firstname:     extract_display_name(article['From']),
           lastname:      '',
           email:         email,
           password:      '',
@@ -53,7 +53,7 @@ module Import
       rescue ActiveRecord::RecordNotUnique
         log "User #{email} was handled by another thread, taking this."
 
-        return if user_found?(article)
+        return if self.class.find(article)
 
         log "User #{email} wasn't created sleep and retry."
         sleep rand 3

lib/import/otrs/article_customer_factory.rb

@@ -4,9 +4,9 @@ module Import
       extend Import::Factory
 
       def skip?(record)
-        return true if record['sender'] != 'customer'
+        return true if record['SenderType'] != 'customer'
-        return true if record['created_by_id'].to_i != 1
+        return true if record['CreatedBy'].to_i != 1
-        return true if record['from'].empty?
+        return true if record['From'].empty?
         false
       end
     end

lib/import/otrs/ticket.rb

@@ -9,7 +9,6 @@ module Import
       MAPPING = {
         Changed: :updated_at,
         Created: :created_at,
-        CreateBy: :created_by_id,
         TicketNumber: :number,
         QueueID: :group_id,
         StateID: :state_id,
@@ -79,7 +78,7 @@ module Import
         {
           owner_id:      owner_id(ticket),
           customer_id:   customer_id(ticket),
-          created_by_id: 1,
+          created_by_id: created_by_id(ticket),
           updated_by_id: 1,
         }
           .merge(from_mapping(ticket))
@@ -119,17 +118,24 @@ module Import
         customer = ticket['CustomerUserID']
 
         return default if !customer
-        user = user_lookup(customer)
 
+        user = user_lookup(customer)
         return user.id if user
 
         first_customer_id = first_customer_id(ticket['Articles'])
-
         return first_customer_id if first_customer_id
 
         default
       end
 
+      def created_by_id(ticket)
+        default = 1
+        return ticket['CreateBy'] if ticket['CreateBy'].to_i != default
+        return default if ticket['Articles'].blank?
+        return default if ticket['Articles'].first['SenderType'] != 'customer'
+        customer_id(ticket)
+      end
+
       def user_lookup(login)
         ::User.find_by(login: login.downcase)
       end
@@ -137,10 +143,11 @@ module Import
       def first_customer_id(articles)
         user_id = nil
         articles.each { |article|
-          next if article['sender'] != 'customer'
+          next if article['SenderType'] != 'customer'
-          next if article['from'].empty?
+          next if article['From'].empty?
-
+          user = Import::OTRS::ArticleCustomer.find(article)
-          user_id = article['created_by_id'].to_i
+          break if !user
+          user_id = user.id
           break
         }
         user_id

spec/import/otrs/article_customer_sepc.rb

@@ -14,26 +14,20 @@ RSpec.describe Import::OTRS::ArticleCustomer do
 
   it 'finds customers by email' do
     expect(import_object).to receive(:find_by).with(email: 'kunde2@kunde.de').and_return(existing_object)
-    expect(existing_object).to receive(:id).and_return(instance_id)
     expect(import_object).not_to receive(:create)
     start_import_test
-    expect(object_structure['created_by_id']).to eq(instance_id)
   end
 
   it 'finds customers by login' do
     expect(import_object).to receive(:find_by).with(email: 'kunde2@kunde.de')
     expect(import_object).to receive(:find_by).with(login: 'kunde2@kunde.de').and_return(existing_object)
-    expect(existing_object).to receive(:id).and_return(instance_id)
     expect(import_object).not_to receive(:create)
     start_import_test
-    expect(object_structure['created_by_id']).to eq(instance_id)
   end
 
   it 'creates customers' do
     expect(import_object).to receive(:find_by).at_least(:once)
     expect(import_object).to receive(:create).and_return(existing_object)
-    expect(existing_object).to receive(:id).and_return(instance_id)
     start_import_test
-    expect(object_structure['created_by_id']).to eq(instance_id)
   end
 end

spec/import/otrs/ticket_spec.rb

@@ -24,7 +24,7 @@ RSpec.describe Import::OTRS::Ticket do
     expect(Import::OTRS::ArticleCustomerFactory).to receive(:import)
     expect(Import::OTRS::ArticleFactory).to receive(:import)
     expect(Import::OTRS::HistoryFactory).to receive(:import)
-    expect(User).to receive(:find_by).twice.and_return(nil)
+    allow(::User).to receive(:find_by).and_return(nil)
     # needed, otherwise 'ActiveRecord::UnknownAttributeError' for
     # DynamicFields will arise
     allow(Import::OTRS::DynamicFieldFactory).to receive('skip_field?').and_return(true)
@@ -97,4 +97,43 @@ RSpec.describe Import::OTRS::Ticket do
       updates_with(zammad_structure)
     end
   end
+
+  context 'unknown customer' do
+
+    let(:object_structure) { load_ticket_json('unknown_customer') }
+    let(:zammad_structure) {
+      {
+        owner_id: 1,
+        customer_id: 1337,
+        created_by_id: 1337,
+        updated_by_id: 1,
+        updated_at: '2014-07-17 14:29:44',
+        created_at: '2014-07-17 14:29:43',
+        number: '2014071754002471',
+        group_id: '2',
+        state_id: '1',
+        priority_id: '3',
+        title: 'Ask me about performance',
+        id: '540'
+      }
+    }
+
+    def article_based_customer_expectation
+      user = instance_double(::User)
+      allow(user).to receive(:id).and_return(1337)
+      allow(Import::OTRS::ArticleCustomer).to receive(:find).with(hash_including('From' => '458@company-sales.com')).and_return(user)
+    end
+
+    it 'creates' do
+      import_backend_expectations
+      article_based_customer_expectation
+      creates_with(zammad_structure)
+    end
+
+    it 'updates' do
+      import_backend_expectations
+      article_based_customer_expectation
+      updates_with(zammad_structure)
+    end
+  end
 end