From aca5edce2c5b4d1539fa21447684a55d5fa0c820 Mon Sep 17 00:00:00 2001 From: Marius BALTEANU Date: Sun, 16 Feb 2020 09:13:05 +0200 Subject: [PATCH 2/2] Auto select fields mapping in import based on the internal field name (ex: estimared_hours, fixed_version, spent_on) or field label (Estimared hours, Version, Date). Some notes: - mappings are case insesitive - 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 --- app/controllers/imports_controller.rb | 34 +++++++- app/models/import.rb | 1 + app/models/issue_import.rb | 17 ++++ app/models/time_entry_import.rb | 9 +++ .../imports/_issues_fields_mapping.html.erb | 3 +- .../files/import_issues_auto_mapping.csv | 2 + test/functional/imports_controller_test.rb | 81 ++++++++++++++++++- 7 files changed, 142 insertions(+), 5 deletions(-) create mode 100644 test/fixtures/files/import_issues_auto_mapping.csv 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' %>

- + <%= mapping_select_tag @import, 'tracker', :required => true, :values => @import.allowed_target_trackers.sorted.map {|t| [t.name, t.id]} %>

@@ -99,4 +99,3 @@

- 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 -- 2.22.0