From: Jean-Philippe Lang Date: Sat, 11 Jun 2016 05:25:46 +0000 (+0000) Subject: Merged r15490 to r15493, r15496, r15497 (#22951). X-Git-Tag: 3.3.0~31 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=8516e1c88dfe3ac2532cb40a539084f194c265b9;p=redmine.git 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 --- 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)} --" : " ".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 @@ -
-

<%= 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' %>

- <%= 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]} %> +

+

+ + <%= mapping_select_tag @import, 'status' %>

+ +
+

<%= 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 @@

<%= l(:label_import_issues) %>

-<% if @import.unsaved_items.count == 0 %> +<% if @import.saved_items.count > 0 %>

<%= l(:notice_import_finished, :count => @import.saved_items.count) %>

-
    +
      <% @import.saved_objects.each do |issue| %>
    • <%= link_to_issue issue %>
    • <% end %>
    -<% else %> +<% end %> + +<% if @import.unsaved_items.count > 0 %>

    <%= l(:notice_import_finished_with_errors, :count => @import.unsaved_items.count, :total => @import.total_items) %>

    + - <% @import.unsaved_items.each do |item| %> + + + <% @import.unsaved_items.each do |item| %> - <% end %> + <% end %> +
    Position Message
    <%= item.position %> <%= simple_format_without_paragraph item.message %>
    <% 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