mirrors
/
redmine
mirror of https://github.com/redmine/redmine.git
Watch 1
Star 0
Fork 0
Code Issues 0 Releases 201 Wiki Activity
Browse Source

Can't set parent issue when issue relations among child issues are present (#13654). 

git-svn-id: http://svn.redmine.org/redmine/trunk@15056 e93f8b46-1217-0410-a6f0-8f06a7374b81
tags/3.3.0
Jean-Philippe Lang 8 years ago
parent
5e6cf80401
commit
3a48f09d4f
3 changed files with 143 additions and 292 deletions
Split View Show Diff Stats
  1. 36
    96
      app/models/issue.rb
  2. 18
    5
      app/models/issue_relation.rb
  3. 89
    191
      test/unit/issue_test.rb

+ 36
- 96
app/models/issue.rb View File

@@ -660,7 +660,10 @@ 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))
elsif (@parent_issue != parent) && (
self.would_reschedule?(@parent_issue) ||
@parent_issue.self_and_ancestors.any? {|a| a.relations_from.any? {|r| r.relation_type == IssueRelation::TYPE_PRECEDES && r.issue_to.would_reschedule?(self)}}
)
errors.add :parent_issue_id, :invalid
elsif !new_record?
# moving an existing issue
@@ -1035,101 +1038,38 @@ class Issue < ActiveRecord::Base
IssueRelation.where("issue_to_id = ? OR issue_from_id = ?", id, id).find(relation_id)
end

# Returns all the other issues that depend on the issue
# The algorithm is a modified breadth first search (bfs)
def all_dependent_issues(except=[])
# The found dependencies
dependencies = []

# The visited flag for every node (issue) used by the breadth first search
eNOT_DISCOVERED = 0 # The issue is "new" to the algorithm, it has not seen it before.

ePROCESS_ALL = 1 # The issue is added to the queue. Process both children and relations of
# the issue when it is processed.

ePROCESS_RELATIONS_ONLY = 2 # The issue was added to the queue and will be output as dependent issue,
# but its children will not be added to the queue when it is processed.

eRELATIONS_PROCESSED = 3 # The related issues, the parent issue and the issue itself have been added to
# the queue, but its children have not been added.

ePROCESS_CHILDREN_ONLY = 4 # The relations and the parent of the issue have been added to the queue, but
# the children still need to be processed.

eALL_PROCESSED = 5 # The issue and all its children, its parent and its related issues have been
# added as dependent issues. It needs no further processing.

issue_status = Hash.new(eNOT_DISCOVERED)

# The queue
queue = []

# Initialize the bfs, add start node (self) to the queue
queue << self
issue_status[self] = ePROCESS_ALL

while (!queue.empty?) do
current_issue = queue.shift
current_issue_status = issue_status[current_issue]
dependencies << current_issue

# Add parent to queue, if not already in it.
parent = current_issue.parent
parent_status = issue_status[parent]

if parent && (parent_status == eNOT_DISCOVERED) && !except.include?(parent)
queue << parent
issue_status[parent] = ePROCESS_RELATIONS_ONLY
end

# Add children to queue, but only if they are not already in it and
# the children of the current node need to be processed.
if (current_issue_status == ePROCESS_CHILDREN_ONLY || current_issue_status == ePROCESS_ALL)
current_issue.children.each do |child|
next if except.include?(child)

if (issue_status[child] == eNOT_DISCOVERED)
queue << child
issue_status[child] = ePROCESS_ALL
elsif (issue_status[child] == eRELATIONS_PROCESSED)
queue << child
issue_status[child] = ePROCESS_CHILDREN_ONLY
elsif (issue_status[child] == ePROCESS_RELATIONS_ONLY)
queue << child
issue_status[child] = ePROCESS_ALL
end
end
end

# Add related issues to the queue, if they are not already in it.
current_issue.relations_from.map(&:issue_to).each do |related_issue|
next if except.include?(related_issue)

if (issue_status[related_issue] == eNOT_DISCOVERED)
queue << related_issue
issue_status[related_issue] = ePROCESS_ALL
elsif (issue_status[related_issue] == eRELATIONS_PROCESSED)
queue << related_issue
issue_status[related_issue] = ePROCESS_CHILDREN_ONLY
elsif (issue_status[related_issue] == ePROCESS_RELATIONS_ONLY)
queue << related_issue
issue_status[related_issue] = ePROCESS_ALL
end
end

# Set new status for current issue
if (current_issue_status == ePROCESS_ALL) || (current_issue_status == ePROCESS_CHILDREN_ONLY)
issue_status[current_issue] = eALL_PROCESSED
elsif (current_issue_status == ePROCESS_RELATIONS_ONLY)
issue_status[current_issue] = eRELATIONS_PROCESSED
end
end # while

# Remove the issues from the "except" parameter from the result array
dependencies -= except
dependencies.delete(self)

dependencies
# Returns true if this issue blocks the other issue, otherwise returns false
def blocks?(other)
all = [self]
last = [self]
while last.any?
current = last.map {|i| i.relations_from.where(:relation_type => IssueRelation::TYPE_BLOCKS).map(&:issue_to)}.flatten.uniq
current -= last
current -= all
return true if current.include?(other)
last = current
all += last
end
false
end

# Returns true if the other issue might be rescheduled if the start/due dates of this issue change
def would_reschedule?(other)
all = [self]
last = [self]
while last.any?
current = last.map {|i|
i.relations_from.where(:relation_type => IssueRelation::TYPE_PRECEDES).map(&:issue_to) +
i.leaves.to_a +
i.ancestors.map {|a| a.relations_from.where(:relation_type => IssueRelation::TYPE_PRECEDES).map(&:issue_to)}
}.flatten.uniq
current -= last
current -= all
return true if current.include?(other)
last = current
all += last
end
false
end

# Returns an array of issues that duplicate this one

+ 18
- 5
app/models/issue_relation.rb View File

@@ -101,11 +101,8 @@ class IssueRelation < ActiveRecord::Base
Setting.cross_project_issue_relations?
errors.add :issue_to_id, :not_same_project
end
# detect circular dependencies depending wether the relation should be reversed
if TYPES.has_key?(relation_type) && TYPES[relation_type][:reverse]
errors.add :base, :circular_dependency if issue_from.all_dependent_issues.include? issue_to
else
errors.add :base, :circular_dependency if issue_to.all_dependent_issues.include? issue_from
if circular_dependency?
errors.add :base, :circular_dependency
end
if issue_from.is_descendant_of?(issue_to) || issue_from.is_ancestor_of?(issue_to)
errors.add :base, :cant_link_an_issue_with_a_descendant
@@ -197,6 +194,22 @@ class IssueRelation < ActiveRecord::Base
end
end

# Returns true if the relation would create a circular dependency
def circular_dependency?
case relation_type
when 'follows'
issue_from.would_reschedule? issue_to
when 'precedes'
issue_to.would_reschedule? issue_from
when 'blocked'
issue_from.blocks? issue_to
when 'blocks'
issue_to.blocks? issue_from
else
false
end
end

def call_issues_relation_added_callback
call_issues_callback :relation_added
end

+ 89
- 191
test/unit/issue_test.rb View File

@@ -1848,20 +1848,49 @@ class IssueTest < ActiveSupport::TestCase
end
end

def test_setting_parent_to_a_dependent_issue_should_not_validate
def test_setting_parent_to_a_an_issue_that_precedes_should_not_validate
# tests that 3 cannot have 1 as parent:
#
# 1 -> 2 -> 3
#
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)
IssueRelation.create!(:issue_from => issue2, :issue_to => issue3, :relation_type => IssueRelation::TYPE_PRECEDES)
issue3.reload
issue3.parent_issue_id = issue2.id
issue3.parent_issue_id = issue1.id
assert !issue3.valid?
assert_include 'Parent task is invalid', issue3.errors.full_messages
end

def test_setting_parent_should_not_allow_circular_dependency
def test_setting_parent_to_a_an_issue_that_follows_should_not_validate
# tests that 1 cannot have 3 as parent:
#
# 1 -> 2 -> 3
#
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 => issue2, :issue_to => issue3, :relation_type => IssueRelation::TYPE_PRECEDES)
issue1.reload
issue1.parent_issue_id = issue3.id
assert !issue1.valid?
assert_include 'Parent task is invalid', issue1.errors.full_messages
end

def test_setting_parent_to_a_an_issue_that_precedes_through_hierarchy_should_not_validate
# tests that 4 cannot have 1 as parent:
# changing the due date of 4 would update the end date of 1 which would reschedule 2
# which would change the end date of 3 which would reschedule 4 and so on...
#
# 3 -> 4
# ^
# 1 -> 2
#
set_language_if_valid 'en'
issue1 = Issue.generate!
issue2 = Issue.generate!
@@ -1878,6 +1907,62 @@ class IssueTest < ActiveSupport::TestCase
assert_include 'Parent task is invalid', issue4.errors.full_messages
end

def test_issue_and_following_issue_should_be_able_to_be_moved_to_the_same_parent
set_language_if_valid 'en'
issue1 = Issue.generate!
issue2 = Issue.generate!
relation = IssueRelation.create!(:issue_from => issue2, :issue_to => issue1, :relation_type => IssueRelation::TYPE_FOLLOWS)
parent = Issue.generate!
issue1.reload.parent_issue_id = parent.id
assert_save issue1
parent.reload
issue2.reload.parent_issue_id = parent.id
assert_save issue2
assert IssueRelation.exists?(relation.id)
end

def test_issue_and_preceding_issue_should_be_able_to_be_moved_to_the_same_parent
set_language_if_valid 'en'
issue1 = Issue.generate!
issue2 = Issue.generate!
relation = IssueRelation.create!(:issue_from => issue2, :issue_to => issue1, :relation_type => IssueRelation::TYPE_PRECEDES)
parent = Issue.generate!
issue1.reload.parent_issue_id = parent.id
assert_save issue1
parent.reload
issue2.reload.parent_issue_id = parent.id
assert_save issue2
assert IssueRelation.exists?(relation.id)
end

def test_issue_and_blocked_issue_should_be_able_to_be_moved_to_the_same_parent
set_language_if_valid 'en'
issue1 = Issue.generate!
issue2 = Issue.generate!
relation = IssueRelation.create!(:issue_from => issue2, :issue_to => issue1, :relation_type => IssueRelation::TYPE_BLOCKED)
parent = Issue.generate!
issue1.reload.parent_issue_id = parent.id
assert_save issue1
parent.reload
issue2.reload.parent_issue_id = parent.id
assert_save issue2
assert IssueRelation.exists?(relation.id)
end

def test_issue_and_blocking_issue_should_be_able_to_be_moved_to_the_same_parent
set_language_if_valid 'en'
issue1 = Issue.generate!
issue2 = Issue.generate!
relation = IssueRelation.create!(:issue_from => issue2, :issue_to => issue1, :relation_type => IssueRelation::TYPE_BLOCKS)
parent = Issue.generate!
issue1.reload.parent_issue_id = parent.id
assert_save issue1
parent.reload
issue2.reload.parent_issue_id = parent.id
assert_save issue2
assert IssueRelation.exists?(relation.id)
end

def test_overdue
assert Issue.new(:due_date => 1.day.ago.to_date).overdue?
assert !Issue.new(:due_date => Date.today).overdue?
@@ -2163,193 +2248,6 @@ class IssueTest < ActiveSupport::TestCase
end
end

def test_all_dependent_issues
IssueRelation.delete_all
assert IssueRelation.create!(:issue_from => Issue.find(1),
:issue_to => Issue.find(2),
:relation_type => IssueRelation::TYPE_PRECEDES)
assert IssueRelation.create!(:issue_from => Issue.find(2),
:issue_to => Issue.find(3),
:relation_type => IssueRelation::TYPE_PRECEDES)
assert IssueRelation.create!(:issue_from => Issue.find(3),
:issue_to => Issue.find(8),
:relation_type => IssueRelation::TYPE_PRECEDES)

assert_equal [2, 3, 8], Issue.find(1).all_dependent_issues.collect(&:id).sort
end

def test_all_dependent_issues_with_subtask
IssueRelation.delete_all

project = Project.generate!(:name => "testproject")

parentIssue = Issue.generate!(:project => project)
childIssue1 = Issue.generate!(:project => project, :parent_issue_id => parentIssue.id)
childIssue2 = Issue.generate!(:project => project, :parent_issue_id => parentIssue.id)

assert_equal [childIssue1.id, childIssue2.id].sort, parentIssue.all_dependent_issues.collect(&:id).uniq.sort
end

def test_all_dependent_issues_does_not_include_self
IssueRelation.delete_all

project = Project.generate!(:name => "testproject")

parentIssue = Issue.generate!(:project => project)
childIssue = Issue.generate!(:project => project, :parent_issue_id => parentIssue.id)

assert_equal [childIssue.id], parentIssue.all_dependent_issues.collect(&:id)
end

def test_all_dependent_issues_with_parenttask_and_sibling
IssueRelation.delete_all

project = Project.generate!(:name => "testproject")

parentIssue = Issue.generate!(:project => project)
childIssue1 = Issue.generate!(:project => project, :parent_issue_id => parentIssue.id)
childIssue2 = Issue.generate!(:project => project, :parent_issue_id => parentIssue.id)

assert_equal [parentIssue.id].sort, childIssue1.all_dependent_issues.collect(&:id)
end

def test_all_dependent_issues_with_relation_to_leaf_in_other_tree
IssueRelation.delete_all

project = Project.generate!(:name => "testproject")

parentIssue1 = Issue.generate!(:project => project)
childIssue1_1 = Issue.generate!(:project => project, :parent_issue_id => parentIssue1.id)
childIssue1_2 = Issue.generate!(:project => project, :parent_issue_id => parentIssue1.id)

parentIssue2 = Issue.generate!(:project => project)
childIssue2_1 = Issue.generate!(:project => project, :parent_issue_id => parentIssue2.id)
childIssue2_2 = Issue.generate!(:project => project, :parent_issue_id => parentIssue2.id)


assert IssueRelation.create(:issue_from => parentIssue1,
:issue_to => childIssue2_2,
:relation_type => IssueRelation::TYPE_BLOCKS)

assert_equal [childIssue1_1.id, childIssue1_2.id, parentIssue2.id, childIssue2_2.id].sort,
parentIssue1.all_dependent_issues.collect(&:id).uniq.sort
end

def test_all_dependent_issues_with_relation_to_parent_in_other_tree
IssueRelation.delete_all

project = Project.generate!(:name => "testproject")

parentIssue1 = Issue.generate!(:project => project)
childIssue1_1 = Issue.generate!(:project => project, :parent_issue_id => parentIssue1.id)
childIssue1_2 = Issue.generate!(:project => project, :parent_issue_id => parentIssue1.id)

parentIssue2 = Issue.generate!(:project => project)
childIssue2_1 = Issue.generate!(:project => project, :parent_issue_id => parentIssue2.id)
childIssue2_2 = Issue.generate!(:project => project, :parent_issue_id => parentIssue2.id)


assert IssueRelation.create(:issue_from => parentIssue1,
:issue_to => parentIssue2,
:relation_type => IssueRelation::TYPE_BLOCKS)

assert_equal [childIssue1_1.id, childIssue1_2.id, parentIssue2.id, childIssue2_1.id, childIssue2_2.id].sort,
parentIssue1.all_dependent_issues.collect(&:id).uniq.sort
end

def test_all_dependent_issues_with_transitive_relation
IssueRelation.delete_all

project = Project.generate!(:name => "testproject")

parentIssue1 = Issue.generate!(:project => project)
childIssue1_1 = Issue.generate!(:project => project, :parent_issue_id => parentIssue1.id)

parentIssue2 = Issue.generate!(:project => project)
childIssue2_1 = Issue.generate!(:project => project, :parent_issue_id => parentIssue2.id)

independentIssue = Issue.generate!(:project => project)

assert IssueRelation.create(:issue_from => parentIssue1,
:issue_to => childIssue2_1,
:relation_type => IssueRelation::TYPE_RELATES)

assert IssueRelation.create(:issue_from => childIssue2_1,
:issue_to => independentIssue,
:relation_type => IssueRelation::TYPE_RELATES)

assert_equal [childIssue1_1.id, parentIssue2.id, childIssue2_1.id, independentIssue.id].sort,
parentIssue1.all_dependent_issues.collect(&:id).uniq.sort
end

def test_all_dependent_issues_with_transitive_relation2
IssueRelation.delete_all

project = Project.generate!(:name => "testproject")

parentIssue1 = Issue.generate!(:project => project)
childIssue1_1 = Issue.generate!(:project => project, :parent_issue_id => parentIssue1.id)

parentIssue2 = Issue.generate!(:project => project)
childIssue2_1 = Issue.generate!(:project => project, :parent_issue_id => parentIssue2.id)

independentIssue = Issue.generate!(:project => project)

assert IssueRelation.create(:issue_from => parentIssue1,
:issue_to => independentIssue,
:relation_type => IssueRelation::TYPE_RELATES)

assert IssueRelation.create(:issue_from => independentIssue,
:issue_to => childIssue2_1,
:relation_type => IssueRelation::TYPE_RELATES)

assert_equal [childIssue1_1.id, parentIssue2.id, childIssue2_1.id, independentIssue.id].sort,
parentIssue1.all_dependent_issues.collect(&:id).uniq.sort

end

def test_all_dependent_issues_with_persistent_circular_dependency
IssueRelation.delete_all
assert IssueRelation.create!(:issue_from => Issue.find(1),
:issue_to => Issue.find(2),
:relation_type => IssueRelation::TYPE_PRECEDES)
assert IssueRelation.create!(:issue_from => Issue.find(2),
:issue_to => Issue.find(3),
:relation_type => IssueRelation::TYPE_PRECEDES)

r = IssueRelation.create!(:issue_from => Issue.find(3),
:issue_to => Issue.find(7),
:relation_type => IssueRelation::TYPE_PRECEDES)
IssueRelation.where(["id = ?", r.id]).update_all("issue_to_id = 1")

assert_equal [2, 3], Issue.find(1).all_dependent_issues.collect(&:id).sort
end

def test_all_dependent_issues_with_persistent_multiple_circular_dependencies
IssueRelation.delete_all
assert IssueRelation.create!(:issue_from => Issue.find(1),
:issue_to => Issue.find(2),
:relation_type => IssueRelation::TYPE_RELATES)
assert IssueRelation.create!(:issue_from => Issue.find(2),
:issue_to => Issue.find(3),
:relation_type => IssueRelation::TYPE_RELATES)
assert IssueRelation.create!(:issue_from => Issue.find(3),
:issue_to => Issue.find(8),
:relation_type => IssueRelation::TYPE_RELATES)

r = IssueRelation.create!(:issue_from => Issue.find(8),
:issue_to => Issue.find(7),
:relation_type => IssueRelation::TYPE_RELATES)
IssueRelation.where(["id = ?", r.id]).update_all("issue_to_id = 2")

r = IssueRelation.create!(:issue_from => Issue.find(3),
:issue_to => Issue.find(7),
:relation_type => IssueRelation::TYPE_RELATES)
IssueRelation.where(["id = ?", r.id]).update_all("issue_to_id = 1")

assert_equal [2, 3, 8], Issue.find(1).all_dependent_issues.collect(&:id).sort
end

test "#done_ratio should use the issue_status according to Setting.issue_done_ratio" do
@issue = Issue.find(1)
@issue_status = IssueStatus.find(1)

Write Preview
Loading…
Cancel
Save