jro_apps/redmine Commit - r6004:f982c5b90d52

Fixed circular dependencies possibly introduced when using reverse relations, for instance "blocked by" relations (#8616)....

Jean-Baptiste Barth -

r6004:f982c5b90d52

parent child

Context file:

r6004:f982c5b90d52

app/models/issue_relation.rb +6 -1

                 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

test/functional/issue_relations_controller_test.rb +2 0

                              :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

test/unit/issue_relation_test.rb +12 0

                 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)
                 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

General Comments 0

You need to be logged in to leave comments. Login now

No TODOs yet

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages