diff --git a/app/models/issue_relation.rb b/app/models/issue_relation.rb index 8f2db40f3..751ac3579 100644 --- a/app/models/issue_relation.rb +++ b/app/models/issue_relation.rb @@ -47,7 +47,12 @@ class IssueRelation < ActiveRecord::Base if issue_from && issue_to errors.add :issue_to_id, :invalid if issue_from_id == issue_to_id errors.add :issue_to_id, :not_same_project unless issue_from.project_id == issue_to.project_id || Setting.cross_project_issue_relations? - errors.add_to_base :circular_dependency if issue_to.all_dependent_issues.include? issue_from + #detect circular dependencies depending wether the relation should be reversed + if TYPES.has_key?(relation_type) && TYPES[relation_type][:reverse] + errors.add_to_base :circular_dependency if issue_from.all_dependent_issues.include? issue_to + else + errors.add_to_base :circular_dependency if issue_to.all_dependent_issues.include? issue_from + end errors.add_to_base :cant_link_an_issue_with_a_descendant if issue_from.is_descendant_of?(issue_to) || issue_from.is_ancestor_of?(issue_to) end end diff --git a/test/functional/issue_relations_controller_test.rb b/test/functional/issue_relations_controller_test.rb index ff3bb8d0d..72f13eb9c 100644 --- a/test/functional/issue_relations_controller_test.rb +++ b/test/functional/issue_relations_controller_test.rb @@ -74,6 +74,8 @@ class IssueRelationsControllerTest < ActionController::TestCase :relation => {:issue_to_id => '4', :relation_type => 'relates', :delay => ''} end end + + should "prevent relation creation when there's a circular dependency" def test_destroy assert_difference 'IssueRelation.count', -1 do diff --git a/test/unit/issue_relation_test.rb b/test/unit/issue_relation_test.rb index f0b8fd666..7f1581a26 100644 --- a/test/unit/issue_relation_test.rb +++ b/test/unit/issue_relation_test.rb @@ -44,6 +44,9 @@ class IssueRelationTest < ActiveSupport::TestCase assert_equal from, relation.issue_to end + # TODO : document why it shouldn't be reversed if validation fails : having + # relations reversed before the validation would allow simpler code for the + # validation def test_follows_relation_should_not_be_reversed_if_validation_fails from = Issue.find(1) to = Issue.find(2) @@ -82,4 +85,13 @@ class IssueRelationTest < ActiveSupport::TestCase assert !r.save assert_not_nil r.errors.on(:base) end + + def test_validates_circular_dependency_on_reverse_relations + IssueRelation.delete_all + assert IssueRelation.create!(:issue_from => Issue.find(1), :issue_to => Issue.find(3), :relation_type => IssueRelation::TYPE_BLOCKS) + assert IssueRelation.create!(:issue_from => Issue.find(1), :issue_to => Issue.find(2), :relation_type => IssueRelation::TYPE_BLOCKED) + r = IssueRelation.new(:issue_from => Issue.find(2), :issue_to => Issue.find(1), :relation_type => IssueRelation::TYPE_BLOCKED) + assert !r.save + assert_not_nil r.errors.on(:base) + end end