Support issue relations when importing issues (#28198).

Patch by Gregor Schmidt and Marius BALTEANU.


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

app/models/import.rb

@@ -212,6 +212,7 @@ class Import < ActiveRecord::Base
         item.save!
         imported += 1
 
+        extend_object(row, item, object) if object.persisted?
         do_callbacks(use_unique_id? ? item.unique_id : item.position, object)
       end
       current = position
@@ -270,7 +271,12 @@ class Import < ActiveRecord::Base
 
   # Builds a record for the given row and returns it
   # To be implemented by subclasses
-  def build_object(row)
+  def build_object(row, item)
+  end
+
+  # Extends object with properties, that may only be handled after it's been
+  # persisted.
+  def extend_object(row, item, object)
   end
 
   # Generates a filename used to store the import file

app/models/issue_import.rb

@@ -230,6 +230,117 @@ class IssueImport < Import
     issue
   end
 
+  def extend_object(row, item, issue)
+    build_relations(row, item, issue)
+  end
+
+  def build_relations(row, item, issue)
+    IssueRelation::TYPES.each_key do |type|
+      has_delay = [IssueRelation::TYPE_PRECEDES, IssueRelation::TYPE_FOLLOWS].include?(type)
+
+      if decls = relation_values(row, "relation_#{type}")
+        decls.each do |decl|
+          unless decl[:matches]
+            # Invalid relation syntax - doesn't match regexp
+            next
+          end
+
+          if decl[:delay] && !has_delay
+            # Invalid relation syntax - delay for relation that doesn't support delays
+            next
+          end
+
+          relation = IssueRelation.new(
+            "relation_type" => type,
+            "issue_from_id" => issue.id
+          )
+
+          if decl[:other_id]
+            relation.issue_to_id = decl[:other_id]
+          elsif decl[:other_pos]
+            if use_unique_id?
+              issue_id = items.where(:unique_id => decl[:other_pos]).first.try(:obj_id)
+              if issue_id
+                relation.issue_to_id = issue_id
+              else
+                add_callback(decl[:other_pos], 'set_relation', item.position, type, decl[:delay])
+                next
+              end
+            elsif decl[:other_pos] > item.position
+              add_callback(decl[:other_pos], 'set_relation', item.position, type, decl[:delay])
+              next
+            elsif issue_id = items.where(:position => decl[:other_pos]).first.try(:obj_id)
+              relation.issue_to_id = issue_id
+            end
+          end
+
+          relation.delay = decl[:delay] if decl[:delay]
+
+          relation.save!
+        end
+      end
+    end
+
+    issue
+  end
+
+  def relation_values(row, name)
+    content = row_value(row, name)
+
+    return if content.blank?
+
+    content.split(",").map do |declaration|
+      declaration = declaration.strip
+
+      # Valid expression:
+      #
+      # 123  => row 123 within the CSV
+      # #123 => issue with ID 123
+      #
+      # For precedes and follows
+      #
+      # 123 7d    => row 123 within CSV with 7 day delay
+      # #123  7d  => issue with ID 123 with 7 day delay
+      # 123 -3d   => negative delay allowed
+      #
+      #
+      # Invalid expression:
+      #
+      # No. 123 => Invalid leading letters
+      # # 123   => Invalid space between # and issue number
+      # 123 8h  => No other time units allowed (just days)
+      #
+      # Please note: If unique_id mapping is present, the whole line - but the
+      # trailing delay expression - is considered unique_id.
+      #
+      # See examples at Rubular http://rubular.com/r/mgXM5Rp6zK
+      #
+      match = declaration.match(/\A(?<unique_id>(?<is_id>#)?(?<id>\d+)|.+?)(?:\s+(?<delay>-?\d+)d)?\z/)
+
+      result = {
+        :matches     => false,
+        :declaration => declaration
+      }
+
+      if match
+        result[:matches] = true
+        result[:delay]   = match[:delay]
+
+        if match[:is_id] && match[:id]
+          result[:other_id] = match[:id]
+        elsif use_unique_id? && match[:unique_id]
+          result[:other_pos] = match[:unique_id]
+        elsif match[:id]
+          result[:other_pos] = match[:id].to_i
+        else
+          result[:matches] = false
+        end
+      end
+
+      result
+    end
+  end
+
   # Callback that sets issue as the parent of a previously imported issue
   def set_as_parent_callback(issue, child_position)
     child_id = items.where(:position => child_position).first.try(:obj_id)
@@ -242,4 +353,19 @@ class IssueImport < Import
     child.save!
     issue.reload
   end
+
+  def set_relation_callback(to_issue, from_position, type, delay)
+    return if to_issue.new_record?
+
+    from_id = items.where(:position => from_position).first.try(:obj_id)
+    return unless from_id
+
+    IssueRelation.create!(
+      'relation_type' => type,
+      'issue_from_id' => from_id,
+      'issue_to_id'   => to_issue.id,
+      'delay'         => delay
+    )
+    to_issue.reload
+  end
 end

app/views/imports/_issues_fields_mapping.html.erb

@@ -1,5 +1,3 @@
-<div class="splitcontent">
-<div class="splitcontentleft">
 <p>
   <label for="import_mapping_project_id"><%= l(:label_project) %></label>
   <%= select_tag 'import_settings[mapping][project_id]',
@@ -15,16 +13,6 @@
   <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">
@@ -64,12 +52,6 @@
     </label>
   <% end %>
 </p>
-<% @custom_fields.each do |field| %>
-  <p>
-    <label for="import_mapping_cf_<%= field.id %>"><%= field.name %></label>
-    <%= mapping_select_tag @import, "cf_#{field.id}" %>
-  </p>
-<% end %>
 </div>
 
 <div class="splitcontentright">
@@ -77,10 +59,6 @@
   <label for="import_mapping_is_private"><%= l(:field_is_private) %></label>
   <%= mapping_select_tag @import, 'is_private' %>
 </p>
-<p>
-  <label for="import_mapping_parent_issue_id"><%= l(:field_parent_issue) %></label>
-  <%= mapping_select_tag @import, 'parent_issue_id' %>
-</p>
 <p>
   <label for="import_mapping_start_date"><%= l(:field_start_date) %></label>
   <%= mapping_select_tag @import, 'start_date' %>
@@ -97,5 +75,11 @@
   <label for="import_mapping_done_ratio"><%= l(:field_done_ratio) %></label>
   <%= mapping_select_tag @import, 'done_ratio' %>
 </p>
+<% @custom_fields.each do |field| %>
+  <p>
+    <label for="import_mapping_cf_<%= field.id %>"><%= field.name %></label>
+    <%= mapping_select_tag @import, "cf_#{field.id}" %>
+  </p>
+<% end %>
 </div>
 </div>

app/views/imports/_issues_mapping.html.erb

@@ -5,6 +5,13 @@
   </div>
 </fieldset>
 
+<fieldset class="box tabular collapsible collapsed">
+  <legend onclick="toggleFieldset(this);" class="icon icon-collapsed"><%= l(:label_relations_mapping) %></legend>
+  <div id="relations-mapping" style="display: none;">
+    <%= render :partial => 'issues_relations_mapping' %>
+  </div>
+</fieldset>
+
 <%= javascript_tag do %>
   $('#fields-mapping').on('change', '#import_mapping_project_id, #import_mapping_tracker', function(){
     $.ajax({

app/views/imports/_issues_relations_mapping.html.erb

@@ -0,0 +1,60 @@
+<div class="splitcontent">
+  <div class="splitcontentleft">
+    <p>
+      <label for="import_mapping_unique_id"><%= l(:field_unique_id) %></label>
+      <%= mapping_select_tag @import, 'unique_id' %>
+    </p>
+    <p>
+      <label for="import_settings_mapping_parent_issue_id"><%= l(:field_parent_issue) %></label>
+      <%= mapping_select_tag @import, 'parent_issue_id' %>
+    </p>
+
+    <p>
+      <label for="import_settings_mapping_relation_duplicates"><%= l(:label_duplicates) %></label>
+      <%= mapping_select_tag @import, 'relation_duplicates' %>
+    </p>
+
+    <p>
+      <label for="import_settings_mapping_relation_duplicated"><%= l(:label_duplicated_by) %></label>
+      <%= mapping_select_tag @import, 'relation_duplicated' %>
+    </p>
+
+    <p>
+      <label for="import_settings_mapping_relation_blocks"><%= l(:label_blocks) %></label>
+      <%= mapping_select_tag @import, 'relation_blocks' %>
+    </p>
+
+    <p>
+      <label for="import_settings_mapping_relation_blocked"><%= l(:label_blocked_by) %></label>
+      <%= mapping_select_tag @import, 'relation_blocked' %>
+    </p>
+  </div>
+
+  <div class="splitcontentright">
+    <p></p>
+    <p>
+      <label for="import_settings_mapping_relation_relates"><%= l(:label_relates_to) %></label>
+      <%= mapping_select_tag @import, 'relation_relates' %>
+    </p>
+
+    <p>
+      <label for="import_settings_mapping_relation_precedes"><%= l(:label_precedes) %></label>
+      <%= mapping_select_tag @import, 'relation_precedes' %>
+    </p>
+
+    <p>
+      <label for="import_settings_mapping_relation_follows"><%= l(:label_follows) %></label>
+      <%= mapping_select_tag @import, 'relation_follows' %>
+    </p>
+
+    <p>
+      <label for="import_settings_mapping_relation_copied_to"><%= l(:label_copied_to) %></label>
+      <%= mapping_select_tag @import, 'relation_copied_to' %>
+    </p>
+
+    <p>
+      <label for="import_settings_mapping_relation_copied_from"><%= l(:label_copied_from) %></label>
+      <%= mapping_select_tag @import, 'relation_copied_from' %>
+    </p>
+  </div>
+</div>

config/locales/de.yml

@@ -1189,6 +1189,7 @@ de:
   label_quote_char: Anführungszeichen
   label_double_quote_char: Doppelte Anführungszeichen
   label_fields_mapping: Zuordnung der Felder
+  label_relations_mapping: Zuordnung von Beziehungen
   label_file_content_preview: Inhaltsvorschau
   label_create_missing_values: Ergänze fehlende Werte
   button_import: Importieren

config/locales/en.yml

@@ -1052,6 +1052,7 @@ en:
   label_quote_char: Quote
   label_double_quote_char: Double quote
   label_fields_mapping: Fields mapping
+  label_relations_mapping: Relations mapping
   label_file_content_preview: File content preview
   label_create_missing_values: Create missing values
   label_api: API

test/fixtures/files/import_subtasks.csv

@@ -1,5 +1,5 @@
-row;tracker;subject;parent
-1;bug;Root;
-2;bug;Child 1;1
-3;bug;Grand-child;4
-4;bug;Child 2;1
+row;tracker;subject;parent;simple relation;delayed relation
+1;bug;Root;;;
+2;bug;Child 1;1;1,4;1 2d
+3;bug;Grand-child;4;4;4 -1d
+4;bug;Child 2;1;1;1 1d

test/fixtures/files/import_subtasks_with_relations.csv

@@ -0,0 +1,5 @@
+row;tracker;subject;start;due;parent;follows
+1;bug;2nd Child;2020-01-12;2020-01-20;3;2 1d
+2;bug;1st Child;2020-01-01;2020-01-10;3;
+3;bug;Parent;2020-01-01;2020-01-31;;
+4;bug;3rd Child;2020-01-22;2020-01-31;3;1 1d

test/fixtures/files/import_subtasks_with_unique_id.csv

@@ -1,5 +1,10 @@
-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
+id;tracker;subject;parent;follows
+RED-IV;bug;Grand-child;RED-III;
+RED-III;bug;Child 2;RED-I;RED-II 1d
+RED-II;bug;Child 1;RED-I;
+RED-I;bug;Root;;
+BLUE-I;bug;Root;;
+BLUE-II;bug;Child 1;BLUE-I;
+BLUE-III;bug;Child 2;BLUE-I;BLUE-II 1d
+BLUE-IV;bug;Grand-child;BLUE-III;
+GREEN-II;bug;Thing;#1;#2 3d;

test/unit/issue_import_test.rb

@@ -146,14 +146,131 @@ class IssueImportTest < ActiveSupport::TestCase
     assert_equal child2, grandchild.parent
   end
 
-  def test_backward_and_forward_reference_with_unique_id
+  def test_references_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.settings['mapping'] = {'project_id' => '1', 'unique_id' => '0', 'tracker' => '1', 'subject' => '2', 'parent_issue_id' => '3', 'relation_follows' => '4'}
     import.save!
 
-    root, child1, grandchild, child2 = new_records(Issue, 4) { import.run }
-    assert_equal root, child1.parent
-    assert_equal child2, grandchild.parent
+    red4, red3, red2, red1, blue1, blue2, blue3, blue4, green = new_records(Issue, 9) { import.run }
+
+    # future references
+    assert_equal red1, red2.parent
+    assert_equal red3, red4.parent
+
+    assert IssueRelation.where('issue_from_id' => red2.id, 'issue_to_id' => red3.id, 'delay' => 1, 'relation_type' => 'precedes').present?
+
+    # past references
+    assert_equal blue1, blue2.parent
+    assert_equal blue3, blue4.parent
+
+    assert IssueRelation.where('issue_from_id' => blue2.id, 'issue_to_id' => blue3.id, 'delay' => 1, 'relation_type' => 'precedes').present?
+
+    assert_equal issues(:issues_001), green.parent
+    assert IssueRelation.where('issue_from_id' => issues(:issues_002).id, 'issue_to_id' => green.id, 'delay' => 3, 'relation_type' => 'precedes').present?
+  end
+
+  def test_follow_relation
+    import = generate_import_with_mapping('import_subtasks.csv')
+    import.settings['mapping'] = {'project_id' => '1', 'tracker' => '1', 'subject' => '2', 'relation_relates' => '4'}
+    import.save!
+
+    one, one_one, one_two_one, one_two = new_records(Issue, 4) { import.run }
+    assert_equal 2, one.relations.count
+    assert one.relations.all? { |r| r.relation_type == 'relates' }
+    assert one.relations.any? { |r| r.other_issue(one) == one_one }
+    assert one.relations.any? { |r| r.other_issue(one) == one_two }
+
+    assert_equal 2, one_one.relations.count
+    assert one_one.relations.all? { |r| r.relation_type == 'relates' }
+    assert one_one.relations.any? { |r| r.other_issue(one_one) == one }
+    assert one_one.relations.any? { |r| r.other_issue(one_one) == one_two }
+
+    assert_equal 3, one_two.relations.count
+    assert one_two.relations.all? { |r| r.relation_type == 'relates' }
+    assert one_two.relations.any? { |r| r.other_issue(one_two) == one }
+    assert one_two.relations.any? { |r| r.other_issue(one_two) == one_one }
+    assert one_two.relations.any? { |r| r.other_issue(one_two) == one_two_one }
+
+    assert_equal 1, one_two_one.relations.count
+    assert one_two_one.relations.all? { |r| r.relation_type == 'relates' }
+    assert one_two_one.relations.any? { |r| r.other_issue(one_two_one) == one_two }
+  end
+
+  def test_delayed_relation
+    import = generate_import_with_mapping('import_subtasks.csv')
+    import.settings['mapping'] = {'project_id' => '1', 'tracker' => '1', 'subject' => '2', 'relation_precedes' => '5'}
+    import.save!
+
+    one, one_one, one_two_one, one_two = new_records(Issue, 4) { import.run }
+
+    assert_equal 2, one.relations_to.count
+    assert one.relations_to.all? { |r| r.relation_type == 'precedes' }
+    assert one.relations_to.any? { |r| r.issue_from == one_one && r.delay == 2 }
+    assert one.relations_to.any? { |r| r.issue_from == one_two && r.delay == 1 }
+
+    assert_equal 1, one_one.relations_from.count
+    assert one_one.relations_from.all? { |r| r.relation_type == 'precedes' }
+    assert one_one.relations_from.any? { |r| r.issue_to == one && r.delay == 2 }
+
+    assert_equal 1, one_two.relations_to.count
+    assert one_two.relations_to.all? { |r| r.relation_type == 'precedes' }
+    assert one_two.relations_to.any? { |r| r.issue_from == one_two_one && r.delay == -1 }
+
+    assert_equal 1, one_two.relations_from.count
+    assert one_two.relations_from.all? { |r| r.relation_type == 'precedes' }
+    assert one_two.relations_from.any? { |r| r.issue_to == one && r.delay == 1 }
+
+    assert_equal 1, one_two_one.relations_from.count
+    assert one_two_one.relations_from.all? { |r| r.relation_type == 'precedes' }
+    assert one_two_one.relations_from.any? { |r| r.issue_to == one_two && r.delay == -1 }
+  end
+
+  def test_parent_and_follows_relation
+    import = generate_import_with_mapping('import_subtasks_with_relations.csv')
+    import.settings['mapping'] = {
+      'project_id'       => '1',
+      'tracker'          => '1',
+
+      'subject'          => '2',
+      'start_date'       => '3',
+      'due_date'         => '4',
+      'parent_issue_id'  => '5',
+      'relation_follows' => '6'
+    }
+    import.save!
+
+    second, first, parent, third = assert_difference('IssueRelation.count', 2) { new_records(Issue, 4) { import.run } }
+
+    # Parent relations
+    assert_equal parent, first.parent
+    assert_equal parent, second.parent
+    assert_equal parent, third.parent
+
+    # Issue relations
+    assert IssueRelation.where(
+      :issue_from_id => first.id,
+      :issue_to_id   => second.id,
+      :relation_type => 'precedes',
+      :delay         => 1).present?
+
+    assert IssueRelation.where(
+      :issue_from_id => second.id,
+      :issue_to_id   => third.id,
+      :relation_type => 'precedes',
+      :delay         => 1).present?
+
+    # Checking dates, because they might act weird, when relations are added
+    assert_equal Date.new(2020, 1, 1), parent.start_date
+    assert_equal Date.new(2020, 2, 3), parent.due_date
+
+    assert_equal Date.new(2020, 1, 1), first.start_date
+    assert_equal Date.new(2020, 1, 10), first.due_date
+
+    assert_equal Date.new(2020, 1, 14), second.start_date
+    assert_equal Date.new(2020, 1, 21), second.due_date
+
+    assert_equal Date.new(2020, 1, 23), third.start_date
+    assert_equal Date.new(2020, 2, 3), third.due_date
   end
 
   def test_assignee_should_be_set