Bulk addition of related issues (#33418).

Patch by Dmitry Makurin and Marius BALTEANU.


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

4 files changed, 102 insertions, 12 deletions

diff --git a/app/controllers/issue_relations_controller.rb b/app/controllers/issue_relations_controller.rb
index 1cceba576..14ee4c948 100644
--- a/app/controllers/issue_relations_controller.rb
+++ b/app/controllers/issue_relations_controller.rb
@@ -44,22 +44,29 @@ class IssueRelationsController < ApplicationController
   end
 
   def create
-    @relation = IssueRelation.new
-    @relation.issue_from = @issue
-    @relation.safe_attributes = params[:relation]
-    @relation.init_journals(User.current)
+    saved = false
+    params_relation = params[:relation]
+    unsaved_relations = []
+
+    relation_issues_to_id.each do |issue_to_id|
+      params_relation[:issue_to_id] = issue_to_id
+
+      @relation = IssueRelation.new
+      @relation.issue_from = @issue
+      @relation.safe_attributes = params_relation
+      @relation.init_journals(User.current)
 
-    begin
-      saved = @relation.save
-    rescue ActiveRecord::RecordNotUnique
-      saved = false
-      @relation.errors.add :base, :taken
+      unless saved = @relation.save
+        saved = false
+        unsaved_relations << @relation
+      end
     end
 
     respond_to do |format|
       format.html {redirect_to issue_path(@issue)}
       format.js do
         @relations = @issue.reload.relations.select {|r| r.other_issue(@issue) && r.other_issue(@issue).visible?}
+        @unsaved_relations = unsaved_relations
       end
       format.api do
         if saved
@@ -98,4 +105,12 @@ class IssueRelationsController < ApplicationController
   rescue ActiveRecord::RecordNotFound
     render_404
   end
+
+  def relation_issues_to_id
+    params[:relation].require(:issue_to_id).split(',').reject(&:blank?)
+  rescue ActionController::ParameterMissing => e
+    # We return a empty array just to loop once and return a validation error
+    # ToDo: Find a better method to return an error if the param is missing.
+    ['']
+  end
 end
diff --git a/app/helpers/issue_relations_helper.rb b/app/helpers/issue_relations_helper.rb
index 9c5d2123b..a1ddad824 100644
--- a/app/helpers/issue_relations_helper.rb
+++ b/app/helpers/issue_relations_helper.rb
@@ -22,4 +22,23 @@ module IssueRelationsHelper
     values = IssueRelation::TYPES
     values.keys.sort_by{|k| values[k][:order]}.collect{|k| [l(values[k][:name]), k]}
   end
+
+  def relation_error_messages(relations)
+    messages = {}
+    relations.each do |item|
+      item.errors.full_messages.each do |message|
+        messages[message] ||= []
+        messages[message] << item
+      end
+    end
+
+    messages.map do |message, items|
+      ids = items.map(&:issue_to_id).compact
+      if ids.empty?
+        message
+      else
+        "#{message}: ##{ids.join(', ')}"
+      end
+    end
+  end
 end
diff --git a/app/views/issue_relations/_form.html.erb b/app/views/issue_relations/_form.html.erb
index 3a1018c1b..5987ac7f8 100644
--- a/app/views/issue_relations/_form.html.erb
+++ b/app/views/issue_relations/_form.html.erb
@@ -1,7 +1,16 @@
-<%= error_messages_for 'relation' %>
-
+<% unsaved_relations_ids = '' %>
+<% if @unsaved_relations && @unsaved_relations.any? %>
+  <% unsaved_relations_ids = @unsaved_relations.map(&:issue_to_id).compact.join(", ") %>
+  <div id="errorExplanation">
+    <ul>
+      <% relation_error_messages(@unsaved_relations).each do |message| %>
+        <li><%= message %></li>
+      <% end %>
+    </ul>
+  </div>
+<% end %>
 <p><%= f.select :relation_type, collection_for_relation_type_select, {}, :onchange => "setPredecessorFieldsVisibility();" %>
-<%= l(:label_issue) %> #<%= f.text_field :issue_to_id, :size => 10 %>
+<%= l(:label_issue) %> #<%= f.text_field :issue_to_id, :value => unsaved_relations_ids, :size => 10 %>
 <span id="predecessor_fields" style="display:none;">
 <%= l(:field_delay) %>: <%= f.text_field :delay, :size => 3 %> <%= l(:label_day_plural) %>
 </span>
diff --git a/test/functional/issue_relations_controller_test.rb b/test/functional/issue_relations_controller_test.rb
index 024172adb..1762afdea 100644
--- a/test/functional/issue_relations_controller_test.rb
+++ b/test/functional/issue_relations_controller_test.rb
@@ -216,6 +216,53 @@ class IssueRelationsControllerTest < Redmine::ControllerTest
     assert_include 'Related issue cannot be blank', response.body
   end
 
+  def test_bulk_create_with_multiple_issue_to_id_issues
+    assert_difference 'IssueRelation.count', +3 do
+      post :create, :params => {
+        :issue_id => 1,
+        :relation => {
+          # js autocomplete adds a comma at the end
+          # issue to id should accept both id and hash with id
+          :issue_to_id => '2,3,#7, ',
+          :relation_type => 'relates',
+          :delay => ''
+        }
+      },
+      :xhr => true
+    end
+
+    assert_response :success
+    relations = IssueRelation.where(:issue_from_id => 1, :issue_to_id => [2, 3, 7])
+    assert_equal 3, relations.count
+    # all relations types should be 'relates'
+    relations.map {|r| assert_equal 'relates', r.relation_type}
+
+    # no error messages should be returned in the response
+    assert_not_include 'id=\"errorExplanation\"', response.body
+  end
+
+  def test_bulk_create_should_show_errors
+    assert_difference 'IssueRelation.count', +3 do
+      post :create, :params => {
+        :issue_id => 1,
+        :relation => {
+          :issue_to_id => '1,2,3,4,5,7',
+          :relation_type => 'relates',
+          :delay => ''
+        }
+      },
+      :xhr => true
+    end
+
+    assert_response :success
+    assert_equal 'text/javascript', response.media_type
+    # issue #1 is invalid
+    assert_include 'Related issue is invalid: #1', response.body
+    # issues #4 and #5 can't be related by default
+    assert_include 'Related issue cannot be blank', response.body
+    assert_include 'Related issue doesn&#39;t belong to the same project', response.body
+  end
+
   def test_destroy
     assert_difference 'IssueRelation.count', -1 do
       delete(:destroy, :params => {:id => '2'})