From e3891ebeedae5018d1d4b4424c64ee3bee9b93f7 Mon Sep 17 00:00:00 2001
From: Thorsten Eckel <te@znuny.com>
Date: Wed, 10 May 2017 09:52:28 +0200
Subject: [PATCH] Working on issue #1008 - Improved trace- and debugability for
 skipped LDAP User import entries.

---
 lib/import/ldap/user.rb           | 22 ++++++++-----
 spec/lib/import/ldap/user_spec.rb | 51 +++++++++++++++++++++++--------
 2 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/lib/import/ldap/user.rb b/lib/import/ldap/user.rb
index 17d767ca4..bcfa52100 100644
--- a/lib/import/ldap/user.rb
+++ b/lib/import/ldap/user.rb
@@ -30,19 +30,25 @@ module Import
       end
 
       def create_or_update(resource, *args)
-        return if skip?(resource)
-
         result = nil
-        catch(:no_roles_assigned) do
-          determine_role_ids(resource)
-
-          result = super(resource, *args)
-
+        if skip?(resource)
           ldap_log(
-            action:  "#{action} -> #{@resource.login}",
+            action:  "skipped -> #{@remote_id}",
             status:  'success',
             request: resource,
           )
+        else
+          catch(:no_roles_assigned) do
+            determine_role_ids(resource)
+
+            result = super(resource, *args)
+
+            ldap_log(
+              action:  "#{action} -> #{@resource.login}",
+              status:  'success',
+              request: resource,
+            )
+          end
         end
 
         result
diff --git a/spec/lib/import/ldap/user_spec.rb b/spec/lib/import/ldap/user_spec.rb
index 3048314c4..6e5e8d50a 100644
--- a/spec/lib/import/ldap/user_spec.rb
+++ b/spec/lib/import/ldap/user_spec.rb
@@ -67,19 +67,6 @@ RSpec.describe Import::Ldap::User do
       expect(HttpLog.last.status).to eq('success')
     end
 
-    it 'skips User entries without attributes' do
-
-      skip_entry = build(:ldap_entry)
-
-      skip_entry['uid'] = [uid]
-
-      expect do
-        described_class.new(skip_entry, ldap_config, user_roles, signup_role_ids)
-      end.to not_change {
-        User.count
-      }
-    end
-
     it 'logs failures to HTTP Log' do
       expect_any_instance_of(User).to receive(:save!).and_raise('SOME ERROR')
       described_class.new(user_entry, ldap_config, user_roles, signup_role_ids)
@@ -265,4 +252,42 @@ RSpec.describe Import::Ldap::User do
       end
     end
   end
+
+  context 'skipped' do
+
+    it 'skips entries without login' do
+      skip_entry = build(:ldap_entry)
+      instance   = nil
+
+      expect do
+        instance = described_class.new(skip_entry, ldap_config, user_roles, signup_role_ids)
+      end.to not_change {
+        User.count
+      }
+      expect(instance.action).to eq(:skipped)
+
+    end
+
+    it 'skips entries without attributes' do
+      skip_entry        = build(:ldap_entry)
+      skip_entry['uid'] = [uid]
+      instance          = nil
+
+      expect do
+        instance = described_class.new(skip_entry, ldap_config, user_roles, signup_role_ids)
+      end.to not_change {
+        User.count
+      }
+      expect(instance.action).to eq(:skipped)
+    end
+
+    it 'logs skips to HTTP Log' do
+      skip_entry = build(:ldap_entry)
+      described_class.new(skip_entry, ldap_config, user_roles, signup_role_ids)
+
+      expect(HttpLog.last.status).to eq('success')
+      expect(HttpLog.last.url).to start_with('skipped')
+    end
+
+  end
 end