diff --git a/app/models/issue.rb b/app/models/issue.rb index 8ba261bb1f..a8810611cb 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -1930,6 +1930,13 @@ class Issue < ActiveRecord::Base # Closes duplicates if the issue is being closed def close_duplicates if Setting.close_duplicate_issues? && closing? + # Check that there are no circular dependency of relationships + duplicate_issue_ids = circular_dependency_duplicate_issue_ids(duplicates) + if duplicate_issue_ids.include?(self.id) + self.errors.add :base, :circular_dependency + throw :abort + end + duplicates.each do |duplicate| # Reload is needed in case the duplicate was updated by a previous duplicate duplicate.reload @@ -1946,6 +1953,17 @@ class Issue < ActiveRecord::Base end end + def circular_dependency_duplicate_issue_ids(issues, duplicate_ids = []) + issues.each do |issue| + next if duplicate_ids.include?(issue.id) + + duplicate_ids << issue.id + duplicate_ids.concat(circular_dependency_duplicate_issue_ids(issue.duplicates, duplicate_ids)) + duplicate_ids.uniq! + end + duplicate_ids + end + # Make sure updated_on is updated when adding a note and set updated_on now # so we can set closed_on with the same value on closing def force_updated_on_change diff --git a/app/models/issue_relation.rb b/app/models/issue_relation.rb index 808686a162..a3dcffdf3b 100644 --- a/app/models/issue_relation.rb +++ b/app/models/issue_relation.rb @@ -238,6 +238,10 @@ class IssueRelation < ActiveRecord::Base issue_from.blocks? issue_to when 'blocks' issue_to.blocks? issue_from + when 'duplicated' + self.class.exists?(issue_from_id: issue_from, issue_to_id: issue_to, relation_type: TYPE_DUPLICATES) + when 'duplicates' + self.class.exists?(issue_from_id: issue_to, issue_to_id: issue_from, relation_type: TYPE_DUPLICATES) when 'relates' self.class.where(issue_from_id: issue_to, issue_to_id: issue_from).present? else diff --git a/test/unit/issue_relation_test.rb b/test/unit/issue_relation_test.rb index f2cf39dcc1..b857a17b8b 100644 --- a/test/unit/issue_relation_test.rb +++ b/test/unit/issue_relation_test.rb @@ -199,6 +199,27 @@ class IssueRelationTest < ActiveSupport::TestCase assert_not_equal [], r.errors[:base] end + def test_validates_circular_dependency_on_reverse_relations_using_duplicates + with_locale 'en' do + IssueRelation.delete_all + issue1 = issues(:issues_001) + issue2 = issues(:issues_002) + assert( + IssueRelation.create!( + :issue_from => issue1, :issue_to => issue2, + :relation_type => IssueRelation::TYPE_DUPLICATES + ) + ) + r = + IssueRelation.new( + :issue_from => issue2, :issue_to => issue1, + :relation_type => IssueRelation::TYPE_DUPLICATES + ) + assert !r.save + assert_include 'This relation would create a circular dependency', r.errors.full_messages + end + end + def test_create_with_initialized_journals_should_create_journals from = Issue.find(1) to = Issue.find(2) diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 95812376a4..d59959c595 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -1615,6 +1615,42 @@ class IssueTest < ActiveSupport::TestCase assert !issue1.reload.closed? end + def test_should_not_close_duplicated_issue_with_multiple_circular_dependencies + IssueRelation.delete_all + issue1 = Issue.generate! + issue2 = Issue.generate! + issue3 = Issue.generate! + + IssueRelation.create(:issue_from => issue1, :issue_to => issue2, + :relation_type => IssueRelation::TYPE_DUPLICATES).save!(:validate => false) + IssueRelation.create(:issue_from => issue2, :issue_to => issue3, + :relation_type => IssueRelation::TYPE_DUPLICATES).save!(:validate => false) + IssueRelation.create(:issue_from => issue3, :issue_to => issue1, + :relation_type => IssueRelation::TYPE_DUPLICATES).save!(:validate => false) + + issue2_a = Issue.generate! + issue2_b = Issue.generate! + + IssueRelation.create(:issue_from => issue2, :issue_to => issue2_a, + :relation_type => IssueRelation::TYPE_DUPLICATES).save!(:validate => false) + IssueRelation.create(:issue_from => issue2_a, :issue_to => issue2_b, + :relation_type => IssueRelation::TYPE_DUPLICATES).save!(:validate => false) + IssueRelation.create(:issue_from => issue2_b, :issue_to => issue2, + :relation_type => IssueRelation::TYPE_DUPLICATES).save!(:validate => false) + + assert_equal 1, issue1.duplicates.count + assert_equal [issue3], issue1.duplicates + assert_equal 1, issue3.duplicates.count + assert_equal [issue2], issue3.duplicates + assert_equal 2, issue2.duplicates.count + assert_equal [issue1, issue2_b], issue2.duplicates.sort + + issue1.init_journal(users(:users_002), "Closing issue") + issue1.status = IssueStatus.where(:is_closed => true).first + assert !issue1.save + assert_include 'This relation would create a circular dependency', issue1.errors.full_messages + end + def test_assignable_versions issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 1, :status_id => 1, :fixed_version_id => 1,