From 17594e9154e843dc800dc0912d0fcadffb3a8b84 Mon Sep 17 00:00:00 2001
From: Marius BALTEANU <marius.balteanu@zitec.com>
Date: Fri, 28 Apr 2017 11:36:26 +0500
Subject: [PATCH 1/9] start working on redmine_people plugin integration

---
 app/controllers/ldap_settings_controller.rb   |  2 +-
 app/helpers/ldap_settings_helper.rb           | 20 ++++++++-
 app/models/ldap_setting.rb                    | 44 +++++++++++++++----
 .../_synchronization_actions.html.erb         |  4 +-
 config/locales/en.yml                         |  1 +
 lib/ldap_sync/entity_manager.rb               |  4 +-
 lib/ldap_sync/infectors.rb                    |  0
 lib/ldap_sync/infectors/auth_source_ldap.rb   |  7 +--
 lib/ldap_sync/infectors/person.rb             | 32 ++++++++++++++
 lib/ldap_sync/infectors/user.rb               |  0
 10 files changed, 97 insertions(+), 17 deletions(-)
 mode change 100644 => 100755 app/controllers/ldap_settings_controller.rb
 mode change 100644 => 100755 app/helpers/ldap_settings_helper.rb
 mode change 100644 => 100755 app/models/ldap_setting.rb
 mode change 100644 => 100755 app/views/ldap_settings/_synchronization_actions.html.erb
 mode change 100644 => 100755 config/locales/en.yml
 mode change 100644 => 100755 lib/ldap_sync/entity_manager.rb
 mode change 100644 => 100755 lib/ldap_sync/infectors.rb
 mode change 100644 => 100755 lib/ldap_sync/infectors/auth_source_ldap.rb
 create mode 100755 lib/ldap_sync/infectors/person.rb
 mode change 100644 => 100755 lib/ldap_sync/infectors/user.rb

diff --git a/app/controllers/ldap_settings_controller.rb b/app/controllers/ldap_settings_controller.rb
old mode 100644
new mode 100755
index cd79f2d..e6d0742
--- a/app/controllers/ldap_settings_controller.rb
+++ b/app/controllers/ldap_settings_controller.rb
@@ -114,7 +114,7 @@ def js_request?
     end
 
     def update_ldap_setting_from_params
-      %w(user group).each do |e|
+      %w(user group person).each do |e|
         params[:ldap_setting]["#{e}_fields_to_sync"] = params["#{e}_fields_to_sync"]
         params[:ldap_setting]["#{e}_ldap_attrs"] = params["#{e}_ldap_attrs"]
       end if params[:ldap_setting]
diff --git a/app/helpers/ldap_settings_helper.rb b/app/helpers/ldap_settings_helper.rb
old mode 100644
new mode 100755
index 1f836ae..6442dfa
--- a/app/helpers/ldap_settings_helper.rb
+++ b/app/helpers/ldap_settings_helper.rb
@@ -91,8 +91,9 @@ def group_fields
 
   def user_fields
     has_user_ldap_attrs = @ldap_setting.has_user_ldap_attrs?
+    standard_fields = User::STANDARD_FIELDS + UserCustomField.all
 
-    (User::STANDARD_FIELDS + UserCustomField.all).map do |f|
+    standard_fields.map do |f|
       if f.is_a?(String)
         id        = f
         name      = l("field_#{f}")
@@ -113,6 +114,23 @@ def user_fields
     end
   end
 
+  def person_fields
+    has_person_ldap_attrs = @ldap_setting.has_person_ldap_attrs?
+    # if Redmine::Plugin.installed?(:redmine_people)
+    # end
+
+    Person::STANDARD_FIELDS.map do |f|
+      SyncField.new(
+        f,
+        l("label_people_#{f}"),
+        false,
+        @ldap_setting.sync_person_fields? && @ldap_setting.person_fields_to_sync.include?(f.to_s),
+        has_person_ldap_attrs ? @ldap_setting.person_ldap_attrs[f.to_s] : '',
+        ''
+      )
+    end
+  end
+
   def options_for_base_settings
     options = [[l(:option_custom), '']]
     options += base_settings.collect {|k, h| [h['name'], k] }.sort
diff --git a/app/models/ldap_setting.rb b/app/models/ldap_setting.rb
old mode 100644
new mode 100755
index 674acc9..73b51f4
--- a/app/models/ldap_setting.rb
+++ b/app/models/ldap_setting.rb
@@ -26,11 +26,11 @@ class LdapSetting
   include ActiveModel::AttributeMethods
 
   # LDAP_DESCRIPTORS
-  LDAP_ATTRIBUTES = %w( groupname member user_memberid user_groups groupid parent_group primary_group group_parentid member_group group_memberid account_flags )
-  CLASS_NAMES = %w( class_user class_group )
+  LDAP_ATTRIBUTES = %w( groupname member user_memberid user_groups groupid parent_group primary_group group_parentid member_group group_memberid account_flags)
+  CLASS_NAMES = %w( class_user class_group class_person )
   FLAGS = %w( create_groups create_users active )
   COMBOS = %w( group_membership nested_groups sync_on_login dyngroups users_search_scope )
-  OTHERS = %w( account_disabled_test user_fields_to_sync group_fields_to_sync user_ldap_attrs group_ldap_attrs fixed_group admin_group required_group group_search_filter groupname_pattern groups_base_dn dyngroups_cache_ttl )
+  OTHERS = %w( account_disabled_test user_fields_to_sync group_fields_to_sync person_fields_to_sync user_ldap_attrs group_ldap_attrs person_ldap_attrs fixed_group admin_group required_group group_search_filter groupname_pattern groups_base_dn dyngroups_cache_ttl )
 
   validates_presence_of :auth_source_ldap_id
   validates_presence_of :class_user, :class_group, :groupname
@@ -55,6 +55,7 @@ class LdapSetting
   validate :validate_group_filter
   validate :validate_user_fields_to_sync, :validate_user_ldap_attrs
   validate :validate_group_fields_to_sync, :validate_group_ldap_attrs
+  validate :validate_person_fields_to_sync, :validate_person_ldap_attrs
 
   before_validation :strip_names, :set_ldap_attrs, :set_fields_to_sync
 
@@ -66,7 +67,7 @@ class LdapSetting
   safe_attributes *(LDAP_ATTRIBUTES + CLASS_NAMES + FLAGS + COMBOS + OTHERS)
   define_attribute_methods LDAP_ATTRIBUTES + CLASS_NAMES + FLAGS + COMBOS + OTHERS
 
-  [:login, *User::STANDARD_FIELDS].each {|f| module_eval("def #{f}; auth_source_ldap.attr_#{f}; end") }
+  [:login, *User::STANDARD_FIELDS].each {|f| module_eval("def #{f}; auth_source_ldap.attr_#{f}; end;");}
 
   def id
     @auth_source_ldap_id
@@ -120,6 +121,10 @@ def sync_group_fields?
     has_group_fields_to_sync?
   end
 
+  def sync_person_fields?
+    has_person_fields_to_sync?
+  end
+
   def sync_dyngroups?
     has_dyngroups?
   end
@@ -157,6 +162,11 @@ def user_ldap_attrs_to_sync(fields = user_fields_to_sync)
     (fields||[]).map {|f| user_ldap_attrs[f] || (send(f.to_sym) if respond_to?(f.to_sym)) }
   end
 
+  # Returns an array of ldap attributes to used when syncing the user fields
+  def person_ldap_attrs_to_sync(fields = person_fields_to_sync)
+    (fields||[]).map {|f| person_ldap_attrs[f] || (send(f.to_sym) if respond_to?(f.to_sym)) }
+  end
+
   # Returns an array of ldap attributes to used when syncing the group fields
   def group_ldap_attrs_to_sync
     (group_fields_to_sync||[]).map {|f| group_ldap_attrs[f] }
@@ -178,7 +188,7 @@ def group_field(ldap_attr)
   def user_field(ldap_attr)
     ldap_attr = ldap_attr.to_s
     result = @user_standard_ldap_attrs.find {|(k, v)| v.downcase == ldap_attr }.try(:first)
-    result ||= user_ldap_attrs.find {|(k, v)| v.downcase == ldap_attr }.try(:first)
+    result ||= (user_ldap_attrs.merge(person_ldap_attrs)).find {|(k, v)| v.downcase == ldap_attr }.try(:first)
   end
 
   def test
@@ -226,7 +236,8 @@ def safe_attributes=(attrs, user = User.current)
 
   def save
     return false if invalid?
-
+    Rails.logger.debug("!"*50)
+    Rails.logger.debug(@attributes)
     self.settings = delete_unsafe_attributes(@attributes, User.current)
   end
 
@@ -308,14 +319,29 @@ def validate_group_fields_to_sync
       validate_fields group_fields_to_sync, GroupCustomField.all, group_ldap_attrs
     end
 
+    def validate_person_ldap_attrs
+      validate_ldap_attrs person_ldap_attrs, Person::STANDARD_FIELDS
+    end
+
+    def validate_person_fields_to_sync
+      validate_fields person_fields_to_sync, Person::STANDARD_FIELDS, person_ldap_attrs
+    end
+
     def validate_ldap_attrs(ldap_attrs, fields)
-      field_ids = fields.map {|f| f.id.to_s }
+      field_ids = fields.map {|f| f.try(:id) ? f.id.to_s : f.to_s }
       ldap_attrs.each do |k, v|
         if !field_ids.include?(k)
           errors.add :user_group_fields, :invalid unless errors.added? :user_group_fields, :invalid
 
         elsif v.present? && v !~ /\A[a-z][a-z0-9-]*\z/i
-          field_name = fields.find {|f| f.id == k.to_i }.name
+          field_name = ''
+          fields.each do |f|
+            if f.try(:id)
+              field_name = f.name if f.id.to_i == k.to_i
+            else
+              field_name = f if f.to_s == k.to_s
+            end
+          end
           errors.add :base, :invalid_ldap_attribute, :field => field_name
         end
       end
@@ -339,11 +365,13 @@ def validate_fields(fields_to_sync, fields, attrs)
     def set_fields_to_sync
       self.user_fields_to_sync ||= []
       self.group_fields_to_sync ||= []
+      self.person_fields_to_sync ||= []
     end
 
     def set_ldap_attrs
       self.user_ldap_attrs ||= {}
       self.group_ldap_attrs ||= {}
+      self.person_ldap_attrs ||= {}
     end
 
     def strip_names
diff --git a/app/views/ldap_settings/_synchronization_actions.html.erb b/app/views/ldap_settings/_synchronization_actions.html.erb
old mode 100644
new mode 100755
index 2ca022d..285f76d
--- a/app/views/ldap_settings/_synchronization_actions.html.erb
+++ b/app/views/ldap_settings/_synchronization_actions.html.erb
@@ -31,7 +31,7 @@
       </tr>
     </thead>
     <tbody>
-      <% { :user => user_fields, :group => group_fields }.each do |k, fields| %>
+      <% { :user => user_fields, :group => group_fields, :person => person_fields }.each do |k, fields| %>
       <% if fields.any? -%>
         <tr class="group open">
           <td colspan="4">
@@ -48,7 +48,7 @@
             <%= check_box_tag "#{k}_fields_to_sync[]", field.id, field.synchronize?, :class => 'sync' %>
           </td>
           <td align="center">
-            <%= text_field_tag "#{k}_ldap_attrs[#{field.id}]", field.ldap_attribute, :size => 12, :disabled => field.id.is_a?(String) %>
+            <%= text_field_tag "#{k}_ldap_attrs[#{field.id}]", field.ldap_attribute, :size => 12, :disabled => (field.id.is_a?(String) && k != :person) %>
           </td>
           <td align="center">
             <%=h field.default_value %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
old mode 100644
new mode 100755
index a41e8c9..77331e5
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -22,6 +22,7 @@ en:
   label_ldap_attributes_on_a_user: "LDAP attributes on a user"
   label_ldap_attributes_on_a_group: "LDAP attributes on a group"
   label_log_messages: "Log messages"
+  label_person: "Person"
 
   button_test: "Test"
   button_execute: "Execute"
diff --git a/lib/ldap_sync/entity_manager.rb b/lib/ldap_sync/entity_manager.rb
old mode 100644
new mode 100755
index 9cae2d7..eacbe33
--- a/lib/ldap_sync/entity_manager.rb
+++ b/lib/ldap_sync/entity_manager.rb
@@ -22,13 +22,13 @@ def connect_as_user?; setting.account.include?('$login'); end
 
   private
     def get_user_fields(username, user_data=nil, options={})
-      fields_to_sync = setting.user_fields_to_sync
+      fields_to_sync = setting.user_fields_to_sync + setting.person_fields_to_sync
       if options.try(:fetch, :include_required, false)
         custom_fields = user_required_custom_fields.map {|cf| cf.id.to_s }
         fields_to_sync -= (User::STANDARD_FIELDS + custom_fields)
         fields_to_sync += (User::STANDARD_FIELDS + custom_fields)
       end
-      ldap_attrs_to_sync = setting.user_ldap_attrs_to_sync(fields_to_sync)
+      ldap_attrs_to_sync = setting.user_ldap_attrs_to_sync(fields_to_sync) + setting.person_ldap_attrs_to_sync(fields_to_sync)
 
       user_data ||= with_ldap_connection do |ldap|
         find_user(ldap, username, ldap_attrs_to_sync)
diff --git a/lib/ldap_sync/infectors.rb b/lib/ldap_sync/infectors.rb
old mode 100644
new mode 100755
diff --git a/lib/ldap_sync/infectors/auth_source_ldap.rb b/lib/ldap_sync/infectors/auth_source_ldap.rb
old mode 100644
new mode 100755
index be06620..afd915f
--- a/lib/ldap_sync/infectors/auth_source_ldap.rb
+++ b/lib/ldap_sync/infectors/auth_source_ldap.rb
@@ -62,7 +62,7 @@ def sync_users
 
       with_ldap_connection do |_|
         ldap_users[:disabled].each do |login|
-          user = self.users.where("LOWER(login) = ?", login.mb_chars.downcase).first
+          user = Person.where("LOWER(login) = ?", login.mb_chars.downcase).first
 
           if user.try(:active?)
             if user.lock!
@@ -96,7 +96,8 @@ def sync_user(user, is_new_user = false, options = {})
         sync_fields = !is_new_user && (!options[:try_to_login] || setting.sync_fields_on_login?)
 
         user_data, flags = if options[:try_to_login] && setting.has_account_flags? && sync_fields
-          user_data = find_user(ldap, user.login, setting.user_ldap_attrs_to_sync + ns(:account_flags))
+          user_attributes = setting.user_ldap_attrs_to_sync + ns(:account_flags) + setting.person_ldap_attrs_to_sync
+          user_data = find_user(ldap, user.login, user_attributes)
           [user_data, user_data.present? ? user_data[n(:account_flags)].first : :deleted]
         end
 
@@ -243,7 +244,7 @@ def find_or_create_group(groupname, group_data = nil)
       end
 
       def find_local_user(username)
-        user = ::User.where("LOWER(#{User.table_name}.login) = ?", username.mb_chars.downcase).first
+        user = ::Person.where("LOWER(login) = ?", username.mb_chars.downcase).first
         if user.present? && user.auth_source_id != self.id
           trace "-- Skipping user '#{user.login}': it already exists on a different auth_source"
           return nil, true
diff --git a/lib/ldap_sync/infectors/person.rb b/lib/ldap_sync/infectors/person.rb
new file mode 100755
index 0000000..a74bf33
--- /dev/null
+++ b/lib/ldap_sync/infectors/person.rb
@@ -0,0 +1,32 @@
+# encoding: utf-8
+# Copyright (C) 2011-2013  The Redmine LDAP Sync Authors
+#
+# This file is part of Redmine LDAP Sync.
+#
+# Redmine LDAP Sync is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Redmine LDAP Sync is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Redmine LDAP Sync.  If not, see <http://www.gnu.org/licenses/>.
+module LdapSync::Infectors::Person
+  ::Person::STANDARD_FIELDS = %w( phone job_title )
+
+  module InstanceMethods
+
+    def synced_fields=(attrs)
+      self.attributes = attrs.slice(*::Person::STANDARD_FIELDS)
+    end
+  end
+
+
+  def self.included(receiver)
+    receiver.send(:include, InstanceMethods)
+  end
+end
\ No newline at end of file
diff --git a/lib/ldap_sync/infectors/user.rb b/lib/ldap_sync/infectors/user.rb
old mode 100644
new mode 100755

From 9950bb09bbcc01dbac6ff3fd043995faf4cab5d9 Mon Sep 17 00:00:00 2001
From: Marius BALTEANU <marius.balteanu@zitec.com>
Date: Fri, 28 Apr 2017 17:04:44 +0000
Subject: [PATCH 2/9] sync person attributes only when the people plugin is
 installed

---
 app/helpers/ldap_settings_helper.rb              |  4 ++--
 app/models/ldap_setting.rb                       |  5 +++--
 lib/ldap_sync/entity_manager.rb                  | 12 ++++++++++--
 lib/ldap_sync/infectors.rb                       |  3 ++-
 lib/ldap_sync/infectors/auth_source_ldap.rb      |  9 +++++----
 lib/ldap_sync/infectors/person.rb                |  7 ++++---
 test/functional/ldap_settings_controller_test.rb |  4 ++--
 7 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/app/helpers/ldap_settings_helper.rb b/app/helpers/ldap_settings_helper.rb
index 6442dfa..5f17660 100755
--- a/app/helpers/ldap_settings_helper.rb
+++ b/app/helpers/ldap_settings_helper.rb
@@ -115,9 +115,9 @@ def user_fields
   end
 
   def person_fields
+    return [] unless Redmine::Plugin.installed?(:redmine_people)
+
     has_person_ldap_attrs = @ldap_setting.has_person_ldap_attrs?
-    # if Redmine::Plugin.installed?(:redmine_people)
-    # end
 
     Person::STANDARD_FIELDS.map do |f|
       SyncField.new(
diff --git a/app/models/ldap_setting.rb b/app/models/ldap_setting.rb
index 73b51f4..ffdaeb8 100755
--- a/app/models/ldap_setting.rb
+++ b/app/models/ldap_setting.rb
@@ -55,7 +55,7 @@ class LdapSetting
   validate :validate_group_filter
   validate :validate_user_fields_to_sync, :validate_user_ldap_attrs
   validate :validate_group_fields_to_sync, :validate_group_ldap_attrs
-  validate :validate_person_fields_to_sync, :validate_person_ldap_attrs
+  validate :validate_person_fields_to_sync, :validate_person_ldap_attrs if Redmine::Plugin.installed?(:redmine_people)
 
   before_validation :strip_names, :set_ldap_attrs, :set_fields_to_sync
 
@@ -187,8 +187,9 @@ def group_field(ldap_attr)
   # Returns the user field name for the given ldap attribute
   def user_field(ldap_attr)
     ldap_attr = ldap_attr.to_s
+    user_ldap_attrs.merge(person_ldap_attrs) if Redmine::Plugin.installed?(:redmine_people)
     result = @user_standard_ldap_attrs.find {|(k, v)| v.downcase == ldap_attr }.try(:first)
-    result ||= (user_ldap_attrs.merge(person_ldap_attrs)).find {|(k, v)| v.downcase == ldap_attr }.try(:first)
+    result ||= (user_ldap_attrs).find {|(k, v)| v.downcase == ldap_attr }.try(:first)
   end
 
   def test
diff --git a/lib/ldap_sync/entity_manager.rb b/lib/ldap_sync/entity_manager.rb
index eacbe33..41100f2 100755
--- a/lib/ldap_sync/entity_manager.rb
+++ b/lib/ldap_sync/entity_manager.rb
@@ -22,13 +22,18 @@ def connect_as_user?; setting.account.include?('$login'); end
 
   private
     def get_user_fields(username, user_data=nil, options={})
-      fields_to_sync = setting.user_fields_to_sync + setting.person_fields_to_sync
+      fields_to_sync = setting.user_fields_to_sync
+      fields_to_sync = fields_to_sync + setting.person_fields_to_sync if Redmine::Plugin.installed?(:usability)
+
       if options.try(:fetch, :include_required, false)
         custom_fields = user_required_custom_fields.map {|cf| cf.id.to_s }
         fields_to_sync -= (User::STANDARD_FIELDS + custom_fields)
         fields_to_sync += (User::STANDARD_FIELDS + custom_fields)
       end
-      ldap_attrs_to_sync = setting.user_ldap_attrs_to_sync(fields_to_sync) + setting.person_ldap_attrs_to_sync(fields_to_sync)
+      ldap_attrs_to_sync = setting.user_ldap_attrs_to_sync(fields_to_sync)
+      ldap_attrs_to_sync = ldap_attrs_to_sync + setting.person_ldap_attrs_to_sync(fields_to_sync) if Redmine::Plugin.installed?(:usability) 
+
+      departments = Department.all.pluck(:id, :name) if Redmine::Plugin.installed?(:redmine_people)
 
       user_data ||= with_ldap_connection do |ldap|
         find_user(ldap, username, ldap_attrs_to_sync)
@@ -38,6 +43,9 @@ def get_user_fields(username, user_data=nil, options={})
       user_fields = user_data.inject({}) do |fields, (attr, value)|
         f = setting.user_field(attr)
         if f && fields_to_sync.include?(f)
+          if (f.to_s == 'department_id' && Redmine::Plugin.installed?(:redmine_people))
+            value, = departments.select {|k,v| v == value.first.to_s} unless value.nil? || value.first.blank?
+          end
           fields[f] = value.first unless value.nil? || value.first.blank?
         end
         fields
diff --git a/lib/ldap_sync/infectors.rb b/lib/ldap_sync/infectors.rb
index 9dd2c1a..908c728 100755
--- a/lib/ldap_sync/infectors.rb
+++ b/lib/ldap_sync/infectors.rb
@@ -1,7 +1,8 @@
 module LdapSync::Infectors
   Dir[File.join(File.dirname(__FILE__), "infectors", "*.rb")].each do |file|
-    require_dependency file;
     infected_name = File.basename(file, ".rb").classify
+    next if infected_name == 'Person' && !Redmine::Plugin.installed?(:usability)  
+    require_dependency file;
     _module = const_get(infected_name)
     _class = Kernel.const_get(infected_name)
     _class.send(:include, _module) unless _class.included_modules.include? _module
diff --git a/lib/ldap_sync/infectors/auth_source_ldap.rb b/lib/ldap_sync/infectors/auth_source_ldap.rb
index afd915f..329def7 100755
--- a/lib/ldap_sync/infectors/auth_source_ldap.rb
+++ b/lib/ldap_sync/infectors/auth_source_ldap.rb
@@ -62,7 +62,8 @@ def sync_users
 
       with_ldap_connection do |_|
         ldap_users[:disabled].each do |login|
-          user = Person.where("LOWER(login) = ?", login.mb_chars.downcase).first
+          klass = Redmine::Plugin.installed?(:redmine_people) ? 'Person' : 'User'          
+          user = Object.const_get(klass).where("LOWER(login) = ?", login.mb_chars.downcase).first
 
           if user.try(:active?)
             if user.lock!
@@ -162,8 +163,7 @@ def sync_user_groups(user)
       end
 
       def sync_user_fields(user, user_data = nil)
-        return unless setting.active? && setting.sync_user_fields?
-
+        return unless setting.active? && (setting.sync_user_fields? || setting.sync_person_fields?)
         user.synced_fields = get_user_fields(user.login, user_data)
 
         if user.save
@@ -244,7 +244,8 @@ def find_or_create_group(groupname, group_data = nil)
       end
 
       def find_local_user(username)
-        user = ::Person.where("LOWER(login) = ?", username.mb_chars.downcase).first
+        klass = Redmine::Plugin.installed?(:redmine_people) ? 'Person' : 'User'
+        user = ::Object.const_get(klass).where("LOWER(login) = ?", username.mb_chars.downcase).first
         if user.present? && user.auth_source_id != self.id
           trace "-- Skipping user '#{user.login}': it already exists on a different auth_source"
           return nil, true
diff --git a/lib/ldap_sync/infectors/person.rb b/lib/ldap_sync/infectors/person.rb
index a74bf33..1fec6bd 100755
--- a/lib/ldap_sync/infectors/person.rb
+++ b/lib/ldap_sync/infectors/person.rb
@@ -16,17 +16,18 @@
 # You should have received a copy of the GNU General Public License
 # along with Redmine LDAP Sync.  If not, see <http://www.gnu.org/licenses/>.
 module LdapSync::Infectors::Person
-  ::Person::STANDARD_FIELDS = %w( phone job_title )
+  ::Person::STANDARD_FIELDS = %w( phone job_title department_id )
 
   module InstanceMethods
 
     def synced_fields=(attrs)
-      self.attributes = attrs.slice(*::Person::STANDARD_FIELDS)
+      self.information_attributes = attrs.slice(*::Person::STANDARD_FIELDS)
     end
+
   end
 
 
   def self.included(receiver)
     receiver.send(:include, InstanceMethods)
   end
-end
\ No newline at end of file
+end
diff --git a/test/functional/ldap_settings_controller_test.rb b/test/functional/ldap_settings_controller_test.rb
index 4ac0c72..2e3a000 100644
--- a/test/functional/ldap_settings_controller_test.rb
+++ b/test/functional/ldap_settings_controller_test.rb
@@ -87,7 +87,7 @@ def test_should_disable_an_invalid_ldap_setting
 
     # We should have
     ldap_setting = LdapSetting.find_by_auth_source_ldap_id(2)
-    assert_equal nil, ldap_setting.member_group, 'LdapSetting is not the same'
+    assert_nil ldap_setting.member_group, 'LdapSetting is not the same'
     assert !ldap_setting.active?, "LdapSetting must be disabled"
   end
 
@@ -222,4 +222,4 @@ def test_should_validate_on_test
 
     assert_no_match /ldap_test\.rb/, response.body, 'Should not throw an error'
   end
-end
\ No newline at end of file
+end

From 791941418c2cd9ca9de89b9ada4141b76f913463 Mon Sep 17 00:00:00 2001
From: Marius BALTEANU <marius.balteanu@zitec.com>
Date: Fri, 28 Apr 2017 17:20:19 +0000
Subject: [PATCH 3/9] revert unnecessary change

---
 app/helpers/ldap_settings_helper.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/helpers/ldap_settings_helper.rb b/app/helpers/ldap_settings_helper.rb
index 5f17660..0448d70 100755
--- a/app/helpers/ldap_settings_helper.rb
+++ b/app/helpers/ldap_settings_helper.rb
@@ -91,9 +91,8 @@ def group_fields
 
   def user_fields
     has_user_ldap_attrs = @ldap_setting.has_user_ldap_attrs?
-    standard_fields = User::STANDARD_FIELDS + UserCustomField.all
 
-    standard_fields.map do |f|
+    (User::STANDARD_FIELDS + UserCustomField.all).map do |f|
       if f.is_a?(String)
         id        = f
         name      = l("field_#{f}")

From 77f21a3cb14fd0127524c28b88f5d738f21c1a8f Mon Sep 17 00:00:00 2001
From: Marius BALTEANU <marius.balteanu@zitec.com>
Date: Fri, 28 Apr 2017 17:21:29 +0000
Subject: [PATCH 4/9] revert unnecessary change

---
 app/models/ldap_setting.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/ldap_setting.rb b/app/models/ldap_setting.rb
index ffdaeb8..7b0121e 100755
--- a/app/models/ldap_setting.rb
+++ b/app/models/ldap_setting.rb
@@ -67,7 +67,7 @@ class LdapSetting
   safe_attributes *(LDAP_ATTRIBUTES + CLASS_NAMES + FLAGS + COMBOS + OTHERS)
   define_attribute_methods LDAP_ATTRIBUTES + CLASS_NAMES + FLAGS + COMBOS + OTHERS
 
-  [:login, *User::STANDARD_FIELDS].each {|f| module_eval("def #{f}; auth_source_ldap.attr_#{f}; end;");}
+  [:login, *User::STANDARD_FIELDS].each {|f| module_eval("def #{f}; auth_source_ldap.attr_#{f}; end");}
 
   def id
     @auth_source_ldap_id

From cd5a20c13e42bd84d466753901a073368ab44f53 Mon Sep 17 00:00:00 2001
From: Marius BALTEANU <marius.balteanu@zitec.com>
Date: Sat, 29 Apr 2017 16:28:47 +0500
Subject: [PATCH 5/9] some cleanup and fix plugin name

---
 app/models/ldap_setting.rb        | 6 ++----
 config/locales/en.yml             | 1 +
 lib/ldap_sync/entity_manager.rb   | 4 ++--
 lib/ldap_sync/infectors.rb        | 2 +-
 lib/ldap_sync/infectors/person.rb | 1 -
 5 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/app/models/ldap_setting.rb b/app/models/ldap_setting.rb
index 7b0121e..d472530 100755
--- a/app/models/ldap_setting.rb
+++ b/app/models/ldap_setting.rb
@@ -67,7 +67,7 @@ class LdapSetting
   safe_attributes *(LDAP_ATTRIBUTES + CLASS_NAMES + FLAGS + COMBOS + OTHERS)
   define_attribute_methods LDAP_ATTRIBUTES + CLASS_NAMES + FLAGS + COMBOS + OTHERS
 
-  [:login, *User::STANDARD_FIELDS].each {|f| module_eval("def #{f}; auth_source_ldap.attr_#{f}; end");}
+  [:login, *User::STANDARD_FIELDS].each {|f| module_eval("def #{f}; auth_source_ldap.attr_#{f}; end")}
 
   def id
     @auth_source_ldap_id
@@ -189,7 +189,7 @@ def user_field(ldap_attr)
     ldap_attr = ldap_attr.to_s
     user_ldap_attrs.merge(person_ldap_attrs) if Redmine::Plugin.installed?(:redmine_people)
     result = @user_standard_ldap_attrs.find {|(k, v)| v.downcase == ldap_attr }.try(:first)
-    result ||= (user_ldap_attrs).find {|(k, v)| v.downcase == ldap_attr }.try(:first)
+    result ||= user_ldap_attrs.find {|(k, v)| v.downcase == ldap_attr }.try(:first)
   end
 
   def test
@@ -237,8 +237,6 @@ def safe_attributes=(attrs, user = User.current)
 
   def save
     return false if invalid?
-    Rails.logger.debug("!"*50)
-    Rails.logger.debug(@attributes)
     self.settings = delete_unsafe_attributes(@attributes, User.current)
   end
 
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 77331e5..8bf22ae 100755
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -23,6 +23,7 @@ en:
   label_ldap_attributes_on_a_group: "LDAP attributes on a group"
   label_log_messages: "Log messages"
   label_person: "Person"
+  label_people_department_id: "Department"
 
   button_test: "Test"
   button_execute: "Execute"
diff --git a/lib/ldap_sync/entity_manager.rb b/lib/ldap_sync/entity_manager.rb
index 41100f2..0ab9de6 100755
--- a/lib/ldap_sync/entity_manager.rb
+++ b/lib/ldap_sync/entity_manager.rb
@@ -23,7 +23,7 @@ def connect_as_user?; setting.account.include?('$login'); end
   private
     def get_user_fields(username, user_data=nil, options={})
       fields_to_sync = setting.user_fields_to_sync
-      fields_to_sync = fields_to_sync + setting.person_fields_to_sync if Redmine::Plugin.installed?(:usability)
+      fields_to_sync = fields_to_sync + setting.person_fields_to_sync if Redmine::Plugin.installed?(:redmine_people)
 
       if options.try(:fetch, :include_required, false)
         custom_fields = user_required_custom_fields.map {|cf| cf.id.to_s }
@@ -31,7 +31,7 @@ def get_user_fields(username, user_data=nil, options={})
         fields_to_sync += (User::STANDARD_FIELDS + custom_fields)
       end
       ldap_attrs_to_sync = setting.user_ldap_attrs_to_sync(fields_to_sync)
-      ldap_attrs_to_sync = ldap_attrs_to_sync + setting.person_ldap_attrs_to_sync(fields_to_sync) if Redmine::Plugin.installed?(:usability) 
+      ldap_attrs_to_sync = ldap_attrs_to_sync + setting.person_ldap_attrs_to_sync(fields_to_sync) if Redmine::Plugin.installed?(:redmine_people)
 
       departments = Department.all.pluck(:id, :name) if Redmine::Plugin.installed?(:redmine_people)
 
diff --git a/lib/ldap_sync/infectors.rb b/lib/ldap_sync/infectors.rb
index 908c728..a7e20cf 100755
--- a/lib/ldap_sync/infectors.rb
+++ b/lib/ldap_sync/infectors.rb
@@ -1,7 +1,7 @@
 module LdapSync::Infectors
   Dir[File.join(File.dirname(__FILE__), "infectors", "*.rb")].each do |file|
     infected_name = File.basename(file, ".rb").classify
-    next if infected_name == 'Person' && !Redmine::Plugin.installed?(:usability)  
+    next if infected_name == 'Person' && !Redmine::Plugin.installed?(:redmine_people)
     require_dependency file;
     _module = const_get(infected_name)
     _class = Kernel.const_get(infected_name)
diff --git a/lib/ldap_sync/infectors/person.rb b/lib/ldap_sync/infectors/person.rb
index 1fec6bd..d8d60f4 100755
--- a/lib/ldap_sync/infectors/person.rb
+++ b/lib/ldap_sync/infectors/person.rb
@@ -26,7 +26,6 @@ def synced_fields=(attrs)
 
   end
 
-
   def self.included(receiver)
     receiver.send(:include, InstanceMethods)
   end

From 560a108f8ccebbbde5b3739bf8166cc39fc7c746 Mon Sep 17 00:00:00 2001
From: Marius BALTEANU <marius.balteanu@zitec.com>
Date: Sun, 30 Apr 2017 22:22:38 +0500
Subject: [PATCH 6/9] small code improvements

---
 app/models/ldap_setting.rb                  | 5 +++--
 lib/ldap_sync/entity_manager.rb             | 9 ++++++---
 lib/ldap_sync/infectors/auth_source_ldap.rb | 4 +---
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/app/models/ldap_setting.rb b/app/models/ldap_setting.rb
index d472530..022c693 100755
--- a/app/models/ldap_setting.rb
+++ b/app/models/ldap_setting.rb
@@ -162,7 +162,7 @@ def user_ldap_attrs_to_sync(fields = user_fields_to_sync)
     (fields||[]).map {|f| user_ldap_attrs[f] || (send(f.to_sym) if respond_to?(f.to_sym)) }
   end
 
-  # Returns an array of ldap attributes to used when syncing the user fields
+  # Returns an array of ldap attributes to used when syncing the person fields
   def person_ldap_attrs_to_sync(fields = person_fields_to_sync)
     (fields||[]).map {|f| person_ldap_attrs[f] || (send(f.to_sym) if respond_to?(f.to_sym)) }
   end
@@ -187,7 +187,8 @@ def group_field(ldap_attr)
   # Returns the user field name for the given ldap attribute
   def user_field(ldap_attr)
     ldap_attr = ldap_attr.to_s
-    user_ldap_attrs.merge(person_ldap_attrs) if Redmine::Plugin.installed?(:redmine_people)
+    user_ldap_attrs.reverse_merge!(person_ldap_attrs) if Redmine::Plugin.installed?(:redmine_people)
+
     result = @user_standard_ldap_attrs.find {|(k, v)| v.downcase == ldap_attr }.try(:first)
     result ||= user_ldap_attrs.find {|(k, v)| v.downcase == ldap_attr }.try(:first)
   end
diff --git a/lib/ldap_sync/entity_manager.rb b/lib/ldap_sync/entity_manager.rb
index 0ab9de6..853d184 100755
--- a/lib/ldap_sync/entity_manager.rb
+++ b/lib/ldap_sync/entity_manager.rb
@@ -23,7 +23,6 @@ def connect_as_user?; setting.account.include?('$login'); end
   private
     def get_user_fields(username, user_data=nil, options={})
       fields_to_sync = setting.user_fields_to_sync
-      fields_to_sync = fields_to_sync + setting.person_fields_to_sync if Redmine::Plugin.installed?(:redmine_people)
 
       if options.try(:fetch, :include_required, false)
         custom_fields = user_required_custom_fields.map {|cf| cf.id.to_s }
@@ -31,9 +30,13 @@ def get_user_fields(username, user_data=nil, options={})
         fields_to_sync += (User::STANDARD_FIELDS + custom_fields)
       end
       ldap_attrs_to_sync = setting.user_ldap_attrs_to_sync(fields_to_sync)
-      ldap_attrs_to_sync = ldap_attrs_to_sync + setting.person_ldap_attrs_to_sync(fields_to_sync) if Redmine::Plugin.installed?(:redmine_people)
 
-      departments = Department.all.pluck(:id, :name) if Redmine::Plugin.installed?(:redmine_people)
+      if Redmine::Plugin.installed?(:redmine_people)
+        person_fields_to_sync = setting.person_fields_to_sync
+        fields_to_sync = fields_to_sync + person_fields_to_sync
+        ldap_attrs_to_sync = ldap_attrs_to_sync + setting.person_ldap_attrs_to_sync(person_fields_to_sync)
+        departments = Department.all.pluck(:id, :name)
+      end
 
       user_data ||= with_ldap_connection do |ldap|
         find_user(ldap, username, ldap_attrs_to_sync)
diff --git a/lib/ldap_sync/infectors/auth_source_ldap.rb b/lib/ldap_sync/infectors/auth_source_ldap.rb
index 329def7..415f775 100755
--- a/lib/ldap_sync/infectors/auth_source_ldap.rb
+++ b/lib/ldap_sync/infectors/auth_source_ldap.rb
@@ -62,9 +62,7 @@ def sync_users
 
       with_ldap_connection do |_|
         ldap_users[:disabled].each do |login|
-          klass = Redmine::Plugin.installed?(:redmine_people) ? 'Person' : 'User'          
-          user = Object.const_get(klass).where("LOWER(login) = ?", login.mb_chars.downcase).first
-
+          user = User.where("LOWER(login) = ?", login.mb_chars.downcase).first
           if user.try(:active?)
             if user.lock!
               change user.login, "-- Locked active user '#{user.login}' (#{user.name})"

From f12dcf8220a50a1b486d042692c2d66e00045d2e Mon Sep 17 00:00:00 2001
From: Marius Balteanu <marius.balteanu@zitec.com>
Date: Sat, 15 Sep 2018 18:19:39 +0300
Subject: [PATCH 7/9] Sync new workday_length attribute

---
 lib/ldap_sync/infectors/person.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ldap_sync/infectors/person.rb b/lib/ldap_sync/infectors/person.rb
index d8d60f4..b5e1af4 100755
--- a/lib/ldap_sync/infectors/person.rb
+++ b/lib/ldap_sync/infectors/person.rb
@@ -16,7 +16,7 @@
 # You should have received a copy of the GNU General Public License
 # along with Redmine LDAP Sync.  If not, see <http://www.gnu.org/licenses/>.
 module LdapSync::Infectors::Person
-  ::Person::STANDARD_FIELDS = %w( phone job_title department_id )
+  ::Person::STANDARD_FIELDS = %w( phone job_title department_id workday_length )
 
   module InstanceMethods
 

From 161ad1d34ecdc0956918233ba40e9dc0d7bc239b Mon Sep 17 00:00:00 2001
From: Marius BALTEANU <marius.balteanu@zitec.com>
Date: Thu, 18 Oct 2018 14:30:41 +0300
Subject: [PATCH 8/9] fixes for user attributes sync

---
 lib/ldap_sync/infectors/auth_source_ldap.rb | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/lib/ldap_sync/infectors/auth_source_ldap.rb b/lib/ldap_sync/infectors/auth_source_ldap.rb
index 415f775..0681559 100755
--- a/lib/ldap_sync/infectors/auth_source_ldap.rb
+++ b/lib/ldap_sync/infectors/auth_source_ldap.rb
@@ -62,7 +62,7 @@ def sync_users
 
       with_ldap_connection do |_|
         ldap_users[:disabled].each do |login|
-          user = User.where("LOWER(login) = ?", login.mb_chars.downcase).first
+          user = self.users.where("LOWER(login) = ?", login.mb_chars.downcase).first
           if user.try(:active?)
             if user.lock!
               change user.login, "-- Locked active user '#{user.login}' (#{user.name})"
@@ -94,7 +94,7 @@ def sync_user(user, is_new_user = false, options = {})
         sync_groups = !options[:try_to_login] || setting.sync_groups_on_login?
         sync_fields = !is_new_user && (!options[:try_to_login] || setting.sync_fields_on_login?)
 
-        user_data, flags = if options[:try_to_login] && setting.has_account_flags? && sync_fields
+        user_data, flags = if (options[:try_to_login] && setting.has_account_flags?) || sync_fields
           user_attributes = setting.user_ldap_attrs_to_sync + ns(:account_flags) + setting.person_ldap_attrs_to_sync
           user_data = find_user(ldap, user.login, user_attributes)
           [user_data, user_data.present? ? user_data[n(:account_flags)].first : :deleted]
@@ -162,9 +162,11 @@ def sync_user_groups(user)
 
       def sync_user_fields(user, user_data = nil)
         return unless setting.active? && (setting.sync_user_fields? || setting.sync_person_fields?)
-        user.synced_fields = get_user_fields(user.login, user_data)
+        user_fields = get_user_fields(user.login, user_data)
+        user.synced_fields = user_fields
 
         if user.save
+          sync_person_fields(user, user_fields) if Redmine::Plugin.installed?(:redmine_people)
           user
         else
           error_message = if user.email_is_taken
@@ -178,6 +180,15 @@ def sync_user_fields(user, user_data = nil)
         end
       end
 
+      def sync_person_fields(user, user_fields)
+        person = Person.find(user.id)
+        person.synced_fields = user_fields
+
+        unless person.save
+          error "Could not sync person '#{user.login}': \"#{person.errors.full_messages.join('", "')}\""; nil
+        end
+      end
+
       def sync_user_status(user, flags, disabled)
         if flags && (flags == :deleted || account_disabled?(flags))
           user.lock!
@@ -242,8 +253,7 @@ def find_or_create_group(groupname, group_data = nil)
       end
 
       def find_local_user(username)
-        klass = Redmine::Plugin.installed?(:redmine_people) ? 'Person' : 'User'
-        user = ::Object.const_get(klass).where("LOWER(login) = ?", username.mb_chars.downcase).first
+        user = ::User.const_get(klass).where("LOWER(login) = ?", username.mb_chars.downcase).first
         if user.present? && user.auth_source_id != self.id
           trace "-- Skipping user '#{user.login}': it already exists on a different auth_source"
           return nil, true

From 59c2d3e2ffb3cf281eb5eb55ada8a708cede89d9 Mon Sep 17 00:00:00 2001
From: Marius BALTEANU <marius.balteanu@zitec.com>
Date: Thu, 18 Oct 2018 14:41:11 +0300
Subject: [PATCH 9/9] fix for User

---
 lib/ldap_sync/infectors/auth_source_ldap.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ldap_sync/infectors/auth_source_ldap.rb b/lib/ldap_sync/infectors/auth_source_ldap.rb
index 0681559..9040d86 100755
--- a/lib/ldap_sync/infectors/auth_source_ldap.rb
+++ b/lib/ldap_sync/infectors/auth_source_ldap.rb
@@ -253,7 +253,7 @@ def find_or_create_group(groupname, group_data = nil)
       end
 
       def find_local_user(username)
-        user = ::User.const_get(klass).where("LOWER(login) = ?", username.mb_chars.downcase).first
+        user = ::User.where("LOWER(login) = ?", username.mb_chars.downcase).first
         if user.present? && user.auth_source_id != self.id
           trace "-- Skipping user '#{user.login}': it already exists on a different auth_source"
           return nil, true