- Fixed bug: An error message is shown when entering the LDAP integra…

…tion admin interface for the first time.

Refactoring:
- Added documentation and tests to ImportJob.
- Added documentation and tests to Import::Ldap.
- Introduced Import::Base.
- Migrated existing Import::Test modules in tests to new class structure.

app/models/import_job.rb

@@ -18,7 +18,8 @@ class ImportJob < ApplicationModel
   def start
     self.started_at = Time.zone.now
     save
-    name.constantize.new(self)
+    instance = name.constantize.new(self)
+    instance.start
   rescue => e
     Rails.logger.error e
 
@@ -81,7 +82,7 @@ def self.start
   end
 
   # Queues all configured import backends from Setting 'import_backends' as import jobs
-  # that are not yet queued.
+  # that are not yet queued. Backends which are not #queueable? are skipped.
   #
   # @example
   #  ImportJob.queue_registered
@@ -98,6 +99,9 @@ def self.queue_registered
         next
       end
 
+      # skip backends that are not "ready" yet
+      next if !backend.constantize.queueable?
+
       # skip if no entry exists
       # skip if a not finished entry exists
       next if ImportJob.exists?(name: backend, finished_at: nil)

lib/import/base.rb

@@ -0,0 +1,39 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+
+module Import
+  class Base
+
+    # Checks if the able to get queued by the scheduler.
+    #
+    # @example
+    #  Import::ExampleBackend.queueable?
+    #  #=> true
+    #
+    # return [Boolean]
+    def self.queueable?
+      true
+    end
+
+    # Initializes a new instance with a stored reference to the import job.
+    #
+    # @example
+    #  instance = Import::ExampleBackend.new(import_job)
+    #
+    # return [Import::ExampleBackend]
+    def initialize(import_job)
+      @import_job = import_job
+    end
+
+    # Starts the life or dry run import of the backend.
+    #
+    # @example
+    #  instance = Import::ExampleBackend.new(import_job)
+    #
+    # @raise [RuntimeError] Raised if the implementation of this mandatory method is missing
+    #
+    # return [nil]
+    def start
+      raise "Missing implementation if the 'start' method."
+    end
+  end
+end

lib/import/ldap.rb

@@ -1,12 +1,32 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+
 require 'ldap'
 require 'ldap/group'
 
 module Import
-  class Ldap
-
-    def initialize(import_job)
-      @import_job = import_job
+  class Ldap < Import::Base
+
+    # Checks if the integration is activated. Otherwise it won't get queued
+    # since it will display an error which is confusing and wrong.
+    #
+    # @example
+    #  Import::LDAP.queueable?
+    #  #=> true
+    #
+    # return [Boolean]
+    def self.queueable?
+      Setting.get('ldap_integration')
+    end
 
+    # Starts a live or dry run LDAP import.
+    #
+    # @example
+    #  instance = Import::LDAP.new(import_job)
+    #
+    # @raise [RuntimeError] Raised if an import should start but the ldap integration is disabled
+    #
+    # return [nil]
+    def start
       if !Setting.get('ldap_integration') && !@import_job.dry_run
         raise "LDAP integration deactivated, check Setting 'ldap_integration'."
       end

spec/factories/import_job.rb

@@ -0,0 +1,7 @@
+FactoryGirl.define do
+  factory :import_job do
+    name 'Import::Test'
+    payload {}
+    dry_run false
+  end
+end

spec/lib/import/base_resource_spec.rb

@@ -14,7 +14,7 @@
 
     before do
       module Import
-        module Test
+        class Test < Import::Base
           class Group < Import::BaseResource
 
             def import_class

spec/lib/import/base_spec.rb

@@ -0,0 +1,26 @@
+require 'rails_helper'
+require 'lib/import/import_job_backend_examples'
+
+RSpec.describe Import::Base do
+  it_behaves_like 'ImportJob backend'
+
+  describe '#queueable?' do
+
+    it 'returns true by default' do
+      expect(described_class.queueable?).to be true
+    end
+  end
+
+  describe '.start' do
+
+    it 'raises an error if called and not overwritten' do
+
+      import_job = create(:import_job)
+      instance   = described_class.new(import_job)
+
+      expect do
+        instance.start
+      end.to raise_error(RuntimeError)
+    end
+  end
+end

spec/lib/import/import_job_backend_examples.rb

@@ -0,0 +1,23 @@
+RSpec.shared_examples 'ImportJob backend' do
+
+  it 'responds to #queueable?' do
+    expect(described_class).to respond_to(:queueable?)
+  end
+
+  it 'requires an import job instance as parameter' do
+
+    expect do
+      described_class.new
+    end.to raise_error(ArgumentError)
+
+    import_job = create(:import_job)
+    expect do
+      described_class.new(import_job)
+    end.not_to raise_error
+  end
+
+  it 'responds to .start' do
+    import_job = create(:import_job)
+    expect(described_class.new(import_job)).to respond_to(:start)
+  end
+end

spec/lib/import/ldap_spec.rb

@@ -0,0 +1,33 @@
+require 'rails_helper'
+require 'lib/import/import_job_backend_examples'
+
+RSpec.describe Import::Ldap do
+  it_behaves_like 'ImportJob backend'
+
+  describe '#queueable?' do
+
+    it 'is queueable if ldap integration is activated' do
+      expect(Setting).to receive(:get).with('ldap_integration').and_return(true)
+      expect(described_class.queueable?).to be true
+    end
+
+    it "isn't queueable if ldap integration is deactivated" do
+      expect(Setting).to receive(:get).with('ldap_integration').and_return(false)
+      expect(described_class.queueable?).to be false
+    end
+  end
+
+  describe '.start' do
+
+    it 'starts LDAP import resource factories' do
+
+      import_job = create(:import_job)
+      instance   = described_class.new(import_job)
+
+      expect(Setting).to receive(:get).with('ldap_integration').and_return(true)
+      expect(Import::Ldap::UserFactory).to receive(:import)
+
+      instance.start
+    end
+  end
+end

spec/lib/import/model_resource_spec.rb

@@ -4,7 +4,7 @@
 
   before do
     module Import
-      module Test
+      class Test < Import::Base
         class Group < Import::ModelResource
           def source
             'RSpec-Test'

spec/lib/import/statistical_factory_spec.rb

@@ -6,7 +6,7 @@
 
   before do
     module Import
-      module Test
+      class Test < Import::Base
 
         module GroupFactory
           extend Import::StatisticalFactory