From f22ef7e076fed7cc39c0bd24984138650f82b8ba Mon Sep 17 00:00:00 2001 From: Gregor Schmidt Date: Fri, 16 Feb 2018 15:25:03 +0100 Subject: [PATCH] Support external ID when importing issues --- app/models/import.rb | 11 +++++--- app/models/issue_import.rb | 30 +++++++++++++++------- app/views/imports/_fields_mapping.html.erb | 12 +++++++++ config/locales/de.yml | 1 + config/locales/en.yml | 1 + ...20180216132815_add_unique_id_to_import_items.rb | 9 +++++++ .../files/import_subtasks_with_unique_id.csv | 5 ++++ test/unit/issue_import_test.rb | 10 ++++++++ 8 files changed, 67 insertions(+), 12 deletions(-) create mode 100644 db/migrate/20180216132815_add_unique_id_to_import_items.rb create mode 100644 test/fixtures/files/import_subtasks_with_unique_id.csv diff --git a/app/models/import.rb b/app/models/import.rb index d2c53baac..50d154304 100644 --- a/app/models/import.rb +++ b/app/models/import.rb @@ -141,8 +141,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 @@ -174,6 +174,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 @@ -186,7 +187,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 @@ -266,4 +267,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 ad04c0be5..526ac6dea 100644 --- a/app/models/issue_import.rb +++ b/app/models/issue_import.rb @@ -138,18 +138,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/_fields_mapping.html.erb b/app/views/imports/_fields_mapping.html.erb index f59350116..a5c5cc5b8 100644 --- a/app/views/imports/_fields_mapping.html.erb +++ b/app/views/imports/_fields_mapping.html.erb @@ -1,3 +1,5 @@ +
+

<%= select_tag 'import_settings[mapping][project_id]', @@ -13,6 +15,16 @@ <%= mapping_select_tag @import, 'status' %>

+
+ +
+

+

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

+
+
diff --git a/config/locales/de.yml b/config/locales/de.yml index 8eebd8180..47c70ef97 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -390,6 +390,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 3fb86c14c..24c4608e2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -376,6 +376,7 @@ en: field_full_width_layout: Full width layout field_digest: Checksum field_default_assigned_to: Default assignee + field_unique_id: Unique ID setting_app_title: Application title setting_app_subtitle: Application subtitle diff --git a/db/migrate/20180216132815_add_unique_id_to_import_items.rb b/db/migrate/20180216132815_add_unique_id_to_import_items.rb new file mode 100644 index 000000000..5db6075cf --- /dev/null +++ b/db/migrate/20180216132815_add_unique_id_to_import_items.rb @@ -0,0 +1,9 @@ +class AddUniqueIdToImportItems < ActiveRecord::Migration[5.1] + 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 bcbf942ae..8def18342 100644 --- a/test/unit/issue_import_test.rb +++ b/test/unit/issue_import_test.rb @@ -128,6 +128,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') -- 2.14.1