From f2922c84299ac0cfca3ba181c3bbe128d3e599af Mon Sep 17 00:00:00 2001 From: Marius BALTEANU Date: Sun, 6 Dec 2020 02:39:29 +0200 Subject: [PATCH 1/2] Rework patch from #33418 --- app/controllers/issue_relations_controller.rb | 33 +++++++++---- app/helpers/issue_relations_helper.rb | 19 ++++++++ app/views/issue_relations/_form.html.erb | 15 ++++-- .../issue_relations_controller_test.rb | 47 +++++++++++++++++++ 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..7e6224a93 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 {|v| v.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(", ") %> +
+ +
+<% end %>

<%= 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 %> 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't belong to the same project', response.body + end + def test_destroy assert_difference 'IssueRelation.count', -1 do delete(:destroy, :params => {:id => '2'}) -- 2.22.0