From 8516e1c88dfe3ac2532cb40a539084f194c265b9 Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sat, 11 Jun 2016 05:25:46 +0000
Subject: [PATCH] Merged r15490 to r15493, r15496, r15497 (#22951).

git-svn-id: http://svn.redmine.org/redmine/branches/3.3-stable@15500 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/controllers/imports_controller.rb      |  6 +---
 app/helpers/imports_helper.rb              |  6 +++-
 app/models/issue.rb                        |  2 +-
 app/models/issue_import.rb                 | 38 ++++++++++++++++++--
 app/views/imports/_fields_mapping.html.erb | 16 +++++----
 app/views/imports/mapping.html.erb         |  2 +-
 app/views/imports/show.html.erb            | 16 ++++++---
 test/fixtures/files/import_issues.csv      |  8 ++---
 test/functional/imports_controller_test.rb |  3 ++
 test/object_helpers.rb                     |  2 +-
 test/unit/issue_import_test.rb             | 42 +++++++++++++++++++++-
 11 files changed, 113 insertions(+), 28 deletions(-)

diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb
index 79a5ced4b..75d4da933 100644
--- a/app/controllers/imports_controller.rb
+++ b/app/controllers/imports_controller.rb
@@ -57,11 +57,7 @@ class ImportsController < ApplicationController
   end
 
   def mapping
-    issue = Issue.new
-    issue.project = @import.project
-    issue.tracker = @import.tracker
-    @attributes = issue.safe_attribute_names
-    @custom_fields = issue.editable_custom_field_values.map(&:custom_field)
+    @custom_fields = @import.mappable_custom_fields
 
     if request.post?
       respond_to do |format|
diff --git a/app/helpers/imports_helper.rb b/app/helpers/imports_helper.rb
index 2fecfca24..edbf2d13e 100644
--- a/app/helpers/imports_helper.rb
+++ b/app/helpers/imports_helper.rb
@@ -23,12 +23,16 @@ module ImportsHelper
     blank_text = options[:required] ? "-- #{l(:actionview_instancetag_blank_option)} --" : "&nbsp;".html_safe
     tags << content_tag('option', blank_text, :value => '')
     tags << options_for_select(import.columns_options, import.mapping[field])
+    if values = options[:values]
+      tags << content_tag('option', '--', :disabled => true)
+      tags << options_for_select(values.map {|text, value| [text, "value:#{value}"]}, import.mapping[field])
+    end
     tags
   end
 
   def mapping_select_tag(import, field, options={})
     name = "import_settings[mapping][#{field}]"
-    select_tag name, options_for_mapping_select(import, field, options)
+    select_tag name, options_for_mapping_select(import, field, options), :id => "import_mapping_#{field}"
   end
 
   # Returns the options for the date_format setting
diff --git a/app/models/issue.rb b/app/models/issue.rb
index e0b99e4e6..98e348d6a 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -691,7 +691,7 @@ class Issue < ActiveRecord::Base
 
     # Checks that the issue can not be added/moved to a disabled tracker
     if project && (tracker_id_changed? || project_id_changed?)
-      unless project.trackers.include?(tracker)
+      if tracker && !project.trackers.include?(tracker)
         errors.add :tracker_id, :inclusion
       end
     end
diff --git a/app/models/issue_import.rb b/app/models/issue_import.rb
index 5b19ac966..4ecd4b517 100644
--- a/app/models/issue_import.rb
+++ b/app/models/issue_import.rb
@@ -41,8 +41,10 @@ class IssueImport < Import
   end
 
   def tracker
-    tracker_id = mapping['tracker_id'].to_i
-    allowed_target_trackers.find_by_id(tracker_id) || allowed_target_trackers.first
+    if mapping['tracker'].to_s =~ /\Avalue:(\d+)\z/
+      tracker_id = $1.to_i
+      allowed_target_trackers.find_by_id(tracker_id)
+    end
   end
 
   # Returns true if missing categories should be created during the import
@@ -57,6 +59,19 @@ class IssueImport < Import
       mapping['create_versions'] == '1'
   end
 
+  def mappable_custom_fields
+    if tracker
+      issue = Issue.new
+      issue.project = project
+      issue.tracker = tracker
+      issue.editable_custom_field_values(user).map(&:custom_field)
+    elsif project
+      project.all_issue_custom_fields
+    else
+      []
+    end
+  end
+
   private
 
   def build_object(row)
@@ -64,12 +79,24 @@ class IssueImport < Import
     issue.author = user
     issue.notify = false
 
+    tracker_id = nil
+    if tracker
+      tracker_id = tracker.id
+    elsif tracker_name = row_value(row, 'tracker')
+      tracker_id = allowed_target_trackers.named(tracker_name).first.try(:id)
+    end
+
     attributes = {
       'project_id' => mapping['project_id'],
-      'tracker_id' => mapping['tracker_id'],
+      'tracker_id' => tracker_id,
       'subject' => row_value(row, 'subject'),
       'description' => row_value(row, 'description')
     }
+    if status_name = row_value(row, 'status')
+      if status_id = IssueStatus.named(status_name).first.try(:id)
+        attributes['status_id'] = status_id
+      end
+    end
     issue.send :safe_attributes=, attributes, user
 
     attributes = {}
@@ -149,6 +176,11 @@ class IssueImport < Import
     end
 
     issue.send :safe_attributes=, attributes, user
+
+    if issue.tracker_id != tracker_id
+      issue.tracker_id = nil
+    end
+
     issue
   end
 end
diff --git a/app/views/imports/_fields_mapping.html.erb b/app/views/imports/_fields_mapping.html.erb
index bb6467eca..0e1d455fa 100644
--- a/app/views/imports/_fields_mapping.html.erb
+++ b/app/views/imports/_fields_mapping.html.erb
@@ -1,17 +1,21 @@
-<div class="splitcontent">
-<div class="splitcontentleft">
 <p>
   <label><%= l(:label_project) %></label>
   <%= select_tag 'import_settings[mapping][project_id]',
         options_for_select(project_tree_options_for_select(@import.allowed_target_projects, :selected => @import.project)),
-        :id => 'issue_project_id' %>
+        :id => 'import_mapping_project_id' %>
 </p>
 <p>
   <label><%= l(:label_tracker) %></label>
-  <%= select_tag 'import_settings[mapping][tracker_id]',
-        options_for_select(@import.allowed_target_trackers.sorted.map {|t| [t.name, t.id]}, @import.tracker.try(:id)),
-        :id => 'issue_tracker_id' %>
+  <%= mapping_select_tag @import, 'tracker', :required => true,
+        :values => @import.allowed_target_trackers.sorted.map {|t| [t.name, t.id]} %>
+</p>
+<p>
+  <label><%= l(:field_status) %></label>
+  <%= mapping_select_tag @import, 'status' %>
 </p>
+
+<div class="splitcontent">
+<div class="splitcontentleft">
 <p>
   <label><%= l(:field_subject) %></label>
   <%= mapping_select_tag @import, 'subject', :required => true %>
diff --git a/app/views/imports/mapping.html.erb b/app/views/imports/mapping.html.erb
index 283bddb04..2e225d6c2 100644
--- a/app/views/imports/mapping.html.erb
+++ b/app/views/imports/mapping.html.erb
@@ -35,7 +35,7 @@
 
 <%= javascript_tag do %>
 $(document).ready(function() {
-  $('#fields-mapping').on('change', '#issue_project_id, #issue_tracker_id', function(){
+  $('#fields-mapping').on('change', '#import_mapping_project_id, #import_mapping_tracker', function(){
     $.ajax({
       url: '<%= import_mapping_path(@import, :format => 'js') %>',
       type: 'post',
diff --git a/app/views/imports/show.html.erb b/app/views/imports/show.html.erb
index d9848cdb5..a50db99fe 100644
--- a/app/views/imports/show.html.erb
+++ b/app/views/imports/show.html.erb
@@ -1,27 +1,33 @@
 <h2><%= l(:label_import_issues) %></h2>
 
-<% if @import.unsaved_items.count == 0 %>
+<% if @import.saved_items.count > 0 %>
   <p><%= l(:notice_import_finished, :count => @import.saved_items.count) %></p>
 
-  <ul>
+  <ul id="saved-items">
   <% @import.saved_objects.each do |issue| %>
     <li><%= link_to_issue issue %></li>
   <% end %>
   </ul>
-<% else %>
+<% end %>
+
+<% if @import.unsaved_items.count > 0 %>
   <p><%= l(:notice_import_finished_with_errors, :count => @import.unsaved_items.count, :total => @import.total_items) %></p>
 
   <table id="unsaved-items" class="list">
+    <thead>
     <tr>
       <th>Position</th>
       <th>Message</th>
     </tr>
-  <% @import.unsaved_items.each do |item| %>
+    </thead>
+    <tbody>
+    <% @import.unsaved_items.each do |item| %>
     <tr>
       <td><%= item.position %></td>
       <td><%= simple_format_without_paragraph item.message %></td>
     </tr>
-  <% end %>
+    <% end %>
+    </tbody>
   </table>
 <% end %>
 
diff --git a/test/fixtures/files/import_issues.csv b/test/fixtures/files/import_issues.csv
index 918f9fc7e..fa9d22665 100644
--- a/test/fixtures/files/import_issues.csv
+++ b/test/fixtures/files/import_issues.csv
@@ -1,4 +1,4 @@
-priority;subject;description;start_date;due_date;parent;private;progress;custom;version;category;user;estimated_hours
-High;First;First description;2015-07-08;2015-08-25;;no;;PostgreSQL;;New category;dlopper;1
-Normal;Child 1;Child description;;;1;yes;10;MySQL;2.0;New category;;2
-Normal;Child of existing issue;Child description;;;#2;no;20;;2.1;Printing;;3
+priority;subject;description;start_date;due_date;parent;private;progress;custom;version;category;user;estimated_hours;tracker;status
+High;First;First description;2015-07-08;2015-08-25;;no;;PostgreSQL;;New category;dlopper;1;bug;new
+Normal;Child 1;Child description;;;1;yes;10;MySQL;2.0;New category;;2;feature request;new
+Normal;Child of existing issue;Child description;;;#2;no;20;;2.1;Printing;;3;bug;assigned
diff --git a/test/functional/imports_controller_test.rb b/test/functional/imports_controller_test.rb
index 9c8dafa34..5c41c10d7 100644
--- a/test/functional/imports_controller_test.rb
+++ b/test/functional/imports_controller_test.rb
@@ -184,6 +184,8 @@ class ImportsControllerTest < ActionController::TestCase
     get :show, :id => import.to_param
     assert_response :success
     assert_template 'show'
+    assert_select 'ul#saved-items'
+    assert_select 'ul#saved-items li', import.saved_items.count
     assert_select 'table#unsaved-items', 0
   end
 
@@ -197,5 +199,6 @@ class ImportsControllerTest < ActionController::TestCase
     assert_response :success
     assert_template 'show'
     assert_select 'table#unsaved-items'
+    assert_select 'table#unsaved-items tbody tr', import.unsaved_items.count
   end
 end
diff --git a/test/object_helpers.rb b/test/object_helpers.rb
index 2b3327961..7ea4a619a 100644
--- a/test/object_helpers.rb
+++ b/test/object_helpers.rb
@@ -235,7 +235,7 @@ module ObjectHelpers
 
     import.settings = {
       'separator' => ";", 'wrapper' => '"', 'encoding' => "UTF-8",
-      'mapping' => {'project_id' => '1', 'tracker_id' => '2', 'subject' => '1'}
+      'mapping' => {'project_id' => '1', 'tracker' => '13', 'subject' => '1'}
     }
     import.save!
     import
diff --git a/test/unit/issue_import_test.rb b/test/unit/issue_import_test.rb
index 61e100700..88da82c88 100644
--- a/test/unit/issue_import_test.rb
+++ b/test/unit/issue_import_test.rb
@@ -58,6 +58,46 @@ class IssueImportTest < ActiveSupport::TestCase
     assert_equal 'New category', category.name
   end
 
+  def test_mapping_with_fixed_tracker
+    import = generate_import_with_mapping
+    import.mapping.merge!('tracker' => 'value:2')
+    import.save!
+
+    issues = new_records(Issue, 3) { import.run }
+    assert_equal [2], issues.map(&:tracker_id).uniq
+  end
+
+  def test_mapping_with_mapped_tracker
+    import = generate_import_with_mapping
+    import.mapping.merge!('tracker' => '13')
+    import.save!
+
+    issues = new_records(Issue, 3) { import.run }
+    assert_equal [1, 2, 1], issues.map(&:tracker_id)
+  end
+
+  def test_should_not_import_with_default_tracker_when_tracker_is_invalid
+    Tracker.find_by_name('Feature request').update!(:name => 'Feature')
+
+    import = generate_import_with_mapping
+    import.mapping.merge!('tracker' => '13')
+    import.save!
+    import.run
+
+    assert_equal 1, import.unsaved_items.count
+    item = import.unsaved_items.first
+    assert_include "Tracker cannot be blank", item.message
+  end
+
+  def test_status_should_be_set
+    import = generate_import_with_mapping
+    import.mapping.merge!('status' => '14')
+    import.save!
+
+    issues = new_records(Issue, 3) { import.run }
+    assert_equal ['New', 'New', 'Assigned'], issues.map(&:status).map(&:name)
+  end
+
   def test_parent_should_be_set
     import = generate_import_with_mapping
     import.mapping.merge!('parent_issue_id' => '5')
@@ -101,7 +141,7 @@ class IssueImportTest < ActiveSupport::TestCase
     field = IssueCustomField.generate!(:field_format => 'date', :is_for_all => true, :trackers => Tracker.all)
     import = generate_import_with_mapping('import_dates.csv')
     import.settings.merge!('date_format' => Import::DATE_FORMATS[1])
-    import.mapping.merge!('subject' => '0', 'start_date' => '1', 'due_date' => '2', "cf_#{field.id}" => '3')
+    import.mapping.merge!('tracker' => 'value:1', 'subject' => '0', 'start_date' => '1', 'due_date' => '2', "cf_#{field.id}" => '3')
     import.save!
 
     issue = new_record(Issue) { import.run } # only 1 valid issue
-- 
2.39.5