Auto select fields mapping in import based on the internal field name (ex: estimared_hours, fixed_version, spent_on) or field label (Estimated hours, Version, Date) (#22913).

* mappings are case insensitive
* a field is auto mapped only if there is no mapping setting present for that field
* "Current user" default value for User field when the user has permission to log time for other users is override by the auto mapped column

Patch by Haihan Ji, Yuichi HARADA, and Marius BALTEANU.


git-svn-id: http://svn.redmine.org/redmine/trunk@19524 e93f8b46-1217-0410-a6f0-8f06a7374b81

7 files changed, 142 insertions, 5 deletions

diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb
index 633cc232c..96c493a31 100644
--- a/app/controllers/imports_controller.rb
+++ b/app/controllers/imports_controller.rb
@@ -66,7 +66,9 @@ class ImportsController < ApplicationController
   def mapping
     @custom_fields = @import.mappable_custom_fields
 
-    if request.post?
+    if request.get?
+      auto_map_fields
+    elsif request.post?
       respond_to do |format|
         format.html {
           if params[:previous]
@@ -159,4 +161,34 @@ class ImportsController < ApplicationController
         type && type < Import ? type : nil
       end
   end
+
+  def auto_map_fields
+    # Try to auto map fields only when settings['enconding'] is present
+    # otherwhise, the import fails for non UTF-8 files because the headers
+    # cannot be retrieved (Invalid byte sequence in UTF-8)
+    return if @import.settings['encoding'].blank?
+
+    mappings = @import.settings['mapping'] ||= {}
+    headers = @import.headers.map(&:downcase)
+
+    # Core fields
+    import_type::AUTO_MAPPABLE_FIELDS.each do |field_nm, label_nm|
+      next if mappings.include?(field_nm)
+      index = headers.index(field_nm) || headers.index(l(label_nm).downcase)
+      if index
+        mappings[field_nm] = index
+      end
+    end
+
+    # Custom fields
+    @custom_fields.each do |field|
+      field_nm = "cf_#{field.id}"
+      next if mappings.include?(field_nm)
+      index = headers.index(field_nm) || headers.index(field.name.downcase)
+      if index
+        mappings[field_nm] = index
+      end
+    end
+    mappings
+  end
 end
diff --git a/app/models/import.rb b/app/models/import.rb
index 61b3553f3..7f2715bdb 100644
--- a/app/models/import.rb
+++ b/app/models/import.rb
@@ -37,6 +37,7 @@ class Import < ActiveRecord::Base
     '%d.%m.%Y',
     '%d-%m-%Y'
   ]
+  AUTO_MAPPABLE_FIELDS = {}
 
   def self.menu_item
     nil
diff --git a/app/models/issue_import.rb b/app/models/issue_import.rb
index 206c0bfd5..7021f5370 100644
--- a/app/models/issue_import.rb
+++ b/app/models/issue_import.rb
@@ -18,6 +18,23 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 class IssueImport < Import
+  AUTO_MAPPABLE_FIELDS = {
+    'tracker' => 'field_tracker',
+    'subject' => 'field_subject',
+    'description' => 'field_description',
+    'status' => 'field_status',
+    'priority' => 'field_priority',
+    'category' => 'field_category',
+    'assigned_to' => 'field_assigned_to',
+    'fixed_version' => 'field_fixed_version',
+    'is_private' => 'field_is_private',
+    'parent_issue_id' => 'field_parent_issue',
+    'start_date' => 'field_start_date',
+    'due_date' => 'field_due_date',
+    'estimated_hours' => 'field_estimated_hours',
+    'done_ratio' => 'field_done_ratio'
+  }
+
   def self.menu_item
     :issues
   end
diff --git a/app/models/time_entry_import.rb b/app/models/time_entry_import.rb
index fa33b60c5..ca3bd04dc 100644
--- a/app/models/time_entry_import.rb
+++ b/app/models/time_entry_import.rb
@@ -18,6 +18,15 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 class TimeEntryImport < Import
+  AUTO_MAPPABLE_FIELDS = {
+    'activity' => 'field_activity',
+    'user' => 'field_user',
+    'issue_id' => 'field_issue',
+    'spent_on' => 'field_spent_on',
+    'hours' => 'field_hours',
+    'comments' => 'field_comments'
+  }
+
   def self.menu_item
     :time_entries
   end
diff --git a/app/views/imports/_issues_fields_mapping.html.erb b/app/views/imports/_issues_fields_mapping.html.erb
index 05bf92e9a..67c538ab4 100644
--- a/app/views/imports/_issues_fields_mapping.html.erb
+++ b/app/views/imports/_issues_fields_mapping.html.erb
@@ -7,7 +7,7 @@
         :id => 'import_mapping_project_id' %>
 </p>
 <p>
-  <label for="import_mapping_tracker"><%= l(:label_tracker) %></label>
+  <label for="import_mapping_tracker"><%= l(:field_tracker) %></label>
   <%= mapping_select_tag @import, 'tracker', :required => true,
         :values => @import.allowed_target_trackers.sorted.map {|t| [t.name, t.id]} %>
 </p>
@@ -99,4 +99,3 @@
 </p>
 </div>
 </div>
-
diff --git a/test/fixtures/files/import_issues_auto_mapping.csv b/test/fixtures/files/import_issues_auto_mapping.csv
new file mode 100644
index 000000000..49785f7a4
--- /dev/null
+++ b/test/fixtures/files/import_issues_auto_mapping.csv
@@ -0,0 +1,2 @@
+priority;Subject;start_date;parent;private;progress;custom;"target version";category;user;estimated_hours;tracker;status;database;cf_6
+High;First;2015-07-08;;no;;PostgreSQL;;New category;dlopper;1;bug;new;"PostgreSQL, Oracle";2
diff --git a/test/functional/imports_controller_test.rb b/test/functional/imports_controller_test.rb
index 7b5a132fc..19507d2e4 100644
--- a/test/functional/imports_controller_test.rb
+++ b/test/functional/imports_controller_test.rb
@@ -167,6 +167,55 @@ class ImportsControllerTest < Redmine::ControllerTest
     end
   end
 
+  def test_get_mapping_should_auto_map_fields_by_internal_field_name_or_by_label
+    import = generate_import('import_issues_auto_mapping.csv')
+    import.settings = {'separator' => ';', 'wrapper'=> '"', 'encoding' => 'ISO-8859-1'}
+    import.save!
+
+    get :mapping, :params => {
+        :id => import.to_param
+      }
+    assert_response :success
+
+    # 'subject' should be auto selected because
+    #  - 'Subject' exists in the import file
+    #  - mapping is case insensitive
+    assert_select 'select[name=?]', 'import_settings[mapping][subject]' do
+      assert_select 'option[value="1"][selected="selected"]', :text => 'Subject'
+    end
+
+    # 'estimated_hours' should be auto selected because
+    #  - 'estimated_hours' exists in the import file
+    assert_select 'select[name=?]', 'import_settings[mapping][estimated_hours]' do
+      assert_select 'option[value="10"][selected="selected"]', :text => 'estimated_hours'
+    end
+
+    # 'fixed_version' should be auto selected because
+    #  - the translation 'Target version' exists in the import file
+    assert_select 'select[name=?]', 'import_settings[mapping][fixed_version]' do
+      assert_select 'option[value="7"][selected="selected"]', :text => 'target version'
+    end
+
+    # 'assigned_to' should not be auto selected because
+    #  - 'assigned_to' does not exist in the import file
+    assert_select 'select[name=?]', 'import_settings[mapping][assigned_to]' do
+      assert_select 'option[selected="selected"]', 0
+    end
+
+    # Custom field 'Float field' should be auto selected because
+    #  - the internal field name ('cf_6') exists in the import file
+    assert_select 'select[name=?]', 'import_settings[mapping][cf_6]' do
+      assert_select 'option[value="14"][selected="selected"]', :text => 'cf_6'
+    end
+
+    # Custom field 'Database' should be auto selected because
+    #  - field name 'database' exists in the import file
+    #  - mapping is case insensitive
+    assert_select 'select[name=?]', 'import_settings[mapping][cf_1]' do
+      assert_select 'option[value="13"][selected="selected"]', :text => 'database'
+    end
+  end
+
   def test_post_mapping_should_update_mapping
     import = generate_import('import_iso8859-1.csv')
 
@@ -200,10 +249,38 @@ class ImportsControllerTest < Redmine::ControllerTest
 
     assert_response :success
 
-    # 'user_id' field should be available because User#2 has both
+    # Assert auto mapped fields
+    assert_select 'select[name=?]', 'import_settings[mapping][activity]' do
+      assert_select 'option[value="5"][selected="selected"]', :text => 'activity'
+    end
+    # 'user' should be mapped to column 'user' from import file
+    # and not to current user because the auto map has priority
+    assert_select 'select[name=?]', 'import_settings[mapping][user]' do
+      assert_select 'option[value="7"][selected="selected"]', :text => 'user'
+    end
+    assert_select 'select[name=?]', 'import_settings[mapping][cf_10]' do
+      assert_select 'option[value="6"][selected="selected"]', :text => 'overtime'
+    end
+  end
+
+  def test_get_mapping_time_entry_for_user_with_log_time_for_other_users_permission
+    Role.find(1).add_permission! :log_time_for_other_users
+    import = generate_time_entry_import
+    import.settings = {
+      'separator' => ";", 'wrapper' => '"', 'encoding' => "ISO-8859-1",
+      # Do not auto map user in order to allow current user to be auto selected
+      'mapping' => {'user' => nil}
+    }
+    import.save!
+
+    get :mapping, :params => {
+        :id => import.to_param
+      }
+
+    # 'user' field should be available because User#2 has both
     # 'import_time_entries' and 'log_time_for_other_users' permissions
     assert_select 'select[name=?]', 'import_settings[mapping][user]' do
-      # Current user should be the default value
+      # Current user should be the default value if there is not auto map present
       assert_select 'option[value="value:2"][selected]', :text => User.find(2).name
       assert_select 'option[value="value:3"]', :text => User.find(3).name
     end