From 5745a2a2e34ea79edb618b94f6095fb6494beec6 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Tue, 19 Mar 2013 18:40:26 +0000 Subject: [PATCH] Merged r11641 and r11642 from trunk (#8794). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/branches/2.3-stable@11656 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/issue.rb | 16 ++++++------ test/functional/issues_controller_test.rb | 2 ++ test/unit/issue_nested_set_test.rb | 16 ------------ test/unit/issue_relation_test.rb | 22 +++++++++++++++++ test/unit/issue_test.rb | 30 +++++++++++++++++++++++ 5 files changed, 63 insertions(+), 23 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index ec108a016..d93cca8bb 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -576,6 +576,8 @@ class Issue < ActiveRecord::Base elsif @parent_issue if !valid_parent_project?(@parent_issue) errors.add :parent_issue_id, :invalid + elsif (@parent_issue != parent) && (all_dependent_issues.include?(@parent_issue) || @parent_issue.all_dependent_issues.include?(self)) + errors.add :parent_issue_id, :invalid elsif !new_record? # moving an existing issue if @parent_issue.root_id != root_id @@ -851,16 +853,16 @@ class Issue < ActiveRecord::Base IssueRelation.find(relation_id, :conditions => ["issue_to_id = ? OR issue_from_id = ?", id, id]) end + # Returns all the other issues that depend on the issue def all_dependent_issues(except=[]) except << self dependencies = [] - relations_from.each do |relation| - if relation.issue_to && !except.include?(relation.issue_to) - dependencies << relation.issue_to - dependencies += relation.issue_to.all_dependent_issues(except) - end - end - dependencies + dependencies += relations_from.map(&:issue_to) + dependencies += children unless leaf? + dependencies << parent + dependencies.compact! + dependencies -= except + dependencies + dependencies.map {|issue| issue.all_dependent_issues(except)}.flatten end # Returns an array of issues that duplicate this one diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index a9cf46140..59f7a858a 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -3474,6 +3474,8 @@ class IssuesControllerTest < ActionController::TestCase end def test_bulk_update_parent_id + IssueRelation.delete_all + @request.session[:user_id] = 2 post :bulk_update, :ids => [1, 3], :notes => 'Bulk editing parent', diff --git a/test/unit/issue_nested_set_test.rb b/test/unit/issue_nested_set_test.rb index dc0c3d3aa..c3120e82a 100644 --- a/test/unit/issue_nested_set_test.rb +++ b/test/unit/issue_nested_set_test.rb @@ -166,22 +166,6 @@ class IssueNestedSetTest < ActiveSupport::TestCase assert_not_nil child.errors[:parent_issue_id] end - def test_moving_an_issue_should_keep_valid_relations_only - issue1 = Issue.generate! - issue2 = Issue.generate! - issue3 = Issue.generate!(:parent_issue_id => issue2.id) - issue4 = Issue.generate! - r1 = IssueRelation.create!(:issue_from => issue1, :issue_to => issue2, :relation_type => IssueRelation::TYPE_PRECEDES) - r2 = IssueRelation.create!(:issue_from => issue1, :issue_to => issue3, :relation_type => IssueRelation::TYPE_PRECEDES) - r3 = IssueRelation.create!(:issue_from => issue2, :issue_to => issue4, :relation_type => IssueRelation::TYPE_PRECEDES) - issue2.reload - issue2.parent_issue_id = issue1.id - issue2.save! - assert !IssueRelation.exists?(r1.id) - assert !IssueRelation.exists?(r2.id) - assert IssueRelation.exists?(r3.id) - end - def test_destroy_should_destroy_children issue1 = Issue.generate! issue2 = Issue.generate! diff --git a/test/unit/issue_relation_test.rb b/test/unit/issue_relation_test.rb index ab67592e0..f15a74c34 100644 --- a/test/unit/issue_relation_test.rb +++ b/test/unit/issue_relation_test.rb @@ -30,6 +30,8 @@ class IssueRelationTest < ActiveSupport::TestCase :enumerations, :trackers + include Redmine::I18n + def test_create from = Issue.find(1) to = Issue.find(2) @@ -115,6 +117,26 @@ class IssueRelationTest < ActiveSupport::TestCase assert_not_nil r.errors[:base] end + def test_validates_circular_dependency_of_subtask + set_language_if_valid 'en' + issue1 = Issue.generate! + issue2 = Issue.generate! + IssueRelation.create!( + :issue_from => issue1, :issue_to => issue2, + :relation_type => IssueRelation::TYPE_PRECEDES + ) + child = Issue.generate!(:parent_issue_id => issue2.id) + issue1.reload + child.reload + + r = IssueRelation.new( + :issue_from => child, :issue_to => issue1, + :relation_type => IssueRelation::TYPE_PRECEDES + ) + assert !r.save + assert_include 'This relation would create a circular dependency', r.errors.full_messages + end + def test_validates_circular_dependency_on_reverse_relations IssueRelation.delete_all assert IssueRelation.create!( diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index d0f44140b..1b35c0595 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -1528,6 +1528,36 @@ class IssueTest < ActiveSupport::TestCase assert child.save end + def test_setting_parent_to_a_dependent_issue_should_not_validate + set_language_if_valid 'en' + issue1 = Issue.generate! + issue2 = Issue.generate! + issue3 = Issue.generate! + IssueRelation.create!(:issue_from => issue1, :issue_to => issue2, :relation_type => IssueRelation::TYPE_PRECEDES) + IssueRelation.create!(:issue_from => issue3, :issue_to => issue1, :relation_type => IssueRelation::TYPE_PRECEDES) + issue3.reload + issue3.parent_issue_id = issue2.id + assert !issue3.valid? + assert_include 'Parent task is invalid', issue3.errors.full_messages + end + + def test_setting_parent_should_not_allow_circular_dependency + set_language_if_valid 'en' + issue1 = Issue.generate! + issue2 = Issue.generate! + IssueRelation.create!(:issue_from => issue1, :issue_to => issue2, :relation_type => IssueRelation::TYPE_PRECEDES) + issue3 = Issue.generate! + issue2.reload + issue2.parent_issue_id = issue3.id + issue2.save! + issue4 = Issue.generate! + IssueRelation.create!(:issue_from => issue3, :issue_to => issue4, :relation_type => IssueRelation::TYPE_PRECEDES) + issue4.reload + issue4.parent_issue_id = issue1.id + assert !issue4.valid? + assert_include 'Parent task is invalid', issue4.errors.full_messages + end + def test_overdue assert Issue.new(:due_date => 1.day.ago.to_date).overdue? assert !Issue.new(:due_date => Date.today).overdue? -- 2.39.5