Support external ID when importing issues (#28213).

Patch by Gregor Schmidt and Marius BALTEANU.

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

8 files changed, 66 insertions, 12 deletions

diff --git a/app/models/import.rb b/app/models/import.rb
index 102bcde73..696cd4d38 100644
--- a/app/models/import.rb
+++ b/app/models/import.rb
@@ -157,8 +157,8 @@ class Import < ActiveRecord::Base
   # Adds a callback that will be called after the item at given position is imported
   def add_callback(position, name, *args)
     settings['callbacks'] ||= {}
-    settings['callbacks'][position.to_i] ||= []
-    settings['callbacks'][position.to_i] << [name, args]
+    settings['callbacks'][position] ||= []
+    settings['callbacks'][position] << [name, args]
     save!
   end
 
@@ -190,6 +190,7 @@ class Import < ActiveRecord::Base
       if position > resume_after
         item = items.build
         item.position = position
+        item.unique_id = row_value(row, 'unique_id') if use_unique_id?
 
         if object = build_object(row, item)
           if object.save
@@ -202,7 +203,7 @@ class Import < ActiveRecord::Base
         item.save!
         imported += 1
 
-        do_callbacks(item.position, object)
+        do_callbacks(use_unique_id? ? item.unique_id : item.position, object)
       end
       current = position
     end
@@ -283,4 +284,8 @@ class Import < ActiveRecord::Base
   def yes?(value)
     value == lu(user, :general_text_yes) || value == '1'
   end
+
+  def use_unique_id?
+    mapping['unique_id'].present?
+  end
 end
diff --git a/app/models/issue_import.rb b/app/models/issue_import.rb
index 5f99007fe..700d659ba 100644
--- a/app/models/issue_import.rb
+++ b/app/models/issue_import.rb
@@ -150,18 +150,30 @@ class IssueImport < Import
       end
     end
     if parent_issue_id = row_value(row, 'parent_issue_id')
-      if parent_issue_id =~ /\A(#)?(\d+)\z/
-        parent_issue_id = $2.to_i
-        if $1
-          attributes['parent_issue_id'] = parent_issue_id
+      if parent_issue_id.start_with? '#'
+        # refers to existing issue
+        attributes['parent_issue_id'] = parent_issue_id[1..-1]
+      elsif use_unique_id?
+        # refers to other row with unique id
+        issue_id = items.where(:unique_id => parent_issue_id).first.try(:obj_id)
+
+        if issue_id
+          attributes['parent_issue_id'] = issue_id
         else
-          if parent_issue_id > item.position
-            add_callback(parent_issue_id, 'set_as_parent', item.position)
-          elsif issue_id = items.where(:position => parent_issue_id).first.try(:obj_id)
-            attributes['parent_issue_id'] = issue_id
-          end
+          add_callback(parent_issue_id, 'set_as_parent', item.position)
         end
+      elsif parent_issue_id =~ /\A\d+\z/
+        # refers to other row by position
+        parent_issue_id = parent_issue_id.to_i
+
+        if parent_issue_id > item.position
+          add_callback(parent_issue_id, 'set_as_parent', item.position)
+        elsif issue_id = items.where(:position => parent_issue_id).first.try(:obj_id)
+          attributes['parent_issue_id'] = issue_id
+        end
+
       else
+        # Something is odd. Assign parent_issue_id to trigger validation error
         attributes['parent_issue_id'] = parent_issue_id
       end
     end
diff --git a/app/views/imports/_issues_fields_mapping.html.erb b/app/views/imports/_issues_fields_mapping.html.erb
index 542a956ff..05bf92e9a 100644
--- a/app/views/imports/_issues_fields_mapping.html.erb
+++ b/app/views/imports/_issues_fields_mapping.html.erb
@@ -1,3 +1,5 @@
+<div class="splitcontent">
+<div class="splitcontentleft">
 <p>
   <label for="import_mapping_project_id"><%= l(:label_project) %></label>
   <%= select_tag 'import_settings[mapping][project_id]',
@@ -13,6 +15,16 @@
   <label for="import_mapping_status"><%= l(:field_status) %></label>
   <%= mapping_select_tag @import, 'status' %>
 </p>
+</div>
+
+<div class="splitcontentright">
+<p></p>
+<p>
+  <label for="import_mapping_unique_id"><%= l(:field_unique_id) %></label>
+  <%= mapping_select_tag @import, 'unique_id' %>
+</p>
+</div>
+</div>
 
 <div class="splitcontent">
 <div class="splitcontentleft">
diff --git a/config/locales/de.yml b/config/locales/de.yml
index 404d0c936..c3647e7d0 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -392,6 +392,7 @@ de:
   field_warn_on_leaving_unsaved: Vor dem Verlassen einer Seite mit ungesichertem Text im Editor warnen
   field_watcher: Beobachter
   field_default_assigned_to: Standardbearbeiter
+  field_unique_id: Eindeutige ID
 
   general_csv_decimal_separator: ','
   general_csv_encoding: ISO-8859-1
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 0b64e979e..f2833e652 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -384,6 +384,7 @@ en:
   field_default_assigned_to: Default assignee
   field_recently_used_projects: Number of recently used projects in jump box
   field_history_default_tab: Issue's history default tab
+  field_unique_id: Unique ID
 
   setting_app_title: Application title
   setting_welcome_text: Welcome text
diff --git a/db/migrate/20190510070108_add_unique_id_to_import_items.rb b/db/migrate/20190510070108_add_unique_id_to_import_items.rb
new file mode 100644
index 000000000..d1b423692
--- /dev/null
+++ b/db/migrate/20190510070108_add_unique_id_to_import_items.rb
@@ -0,0 +1,8 @@
+class AddUniqueIdToImportItems < ActiveRecord::Migration[5.2]
+  def change
+    change_table :import_items do |t|
+      t.string "unique_id"
+      t.index ["import_id", "unique_id"]
+    end
+  end
+end
diff --git a/test/fixtures/files/import_subtasks_with_unique_id.csv b/test/fixtures/files/import_subtasks_with_unique_id.csv
new file mode 100644
index 000000000..bd01e7298
--- /dev/null
+++ b/test/fixtures/files/import_subtasks_with_unique_id.csv
@@ -0,0 +1,5 @@
+id;tracker;subject;parent
+RED-I;bug;Root;
+RED-II;bug;Child 1;RED-I
+RED-III;bug;Grand-child;RED-IV
+RED-IV;bug;Child 2;RED-I
diff --git a/test/unit/issue_import_test.rb b/test/unit/issue_import_test.rb
index 7f66481fc..8404a0ec1 100644
--- a/test/unit/issue_import_test.rb
+++ b/test/unit/issue_import_test.rb
@@ -146,6 +146,16 @@ class IssueImportTest < ActiveSupport::TestCase
     assert_equal child2, grandchild.parent
   end
 
+  def test_backward_and_forward_reference_with_unique_id
+    import = generate_import_with_mapping('import_subtasks_with_unique_id.csv')
+    import.settings['mapping'] = {'project_id' => '1', 'unique_id' => '0', 'tracker' => '1', 'subject' => '2', 'parent_issue_id' => '3'}
+    import.save!
+
+    root, child1, grandchild, child2 = new_records(Issue, 4) { import.run }
+    assert_equal root, child1.parent
+    assert_equal child2, grandchild.parent
+  end
+
   def test_assignee_should_be_set
     import = generate_import_with_mapping
     import.mapping.merge!('assigned_to' => '11')