##// END OF EJS Templates
jro_apps » redmine
RSS
Fixed circular dependencies possibly introduced when using reverse relations, for instance "blocked by" relations (#8616)....
Jean-Baptiste Barth -
r6004:f982c5b90d52
parent child
Show More
Side by Side Unified
Context file:
r6004:f982c5b90d52 Loading diff...:
Show file before | Show file after | Hide comments
@@ -1,113 +1,118
1 1 # redMine - project management software
2 2 # Copyright (C) 2006-2007 Jean-Philippe Lang
3 3 #
4 4 # This program is free software; you can redistribute it and/or
5 5 # modify it under the terms of the GNU General Public License
6 6 # as published by the Free Software Foundation; either version 2
7 7 # of the License, or (at your option) any later version.
8 8 #
9 9 # This program is distributed in the hope that it will be useful,
10 10 # but WITHOUT ANY WARRANTY; without even the implied warranty of
11 11 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12 12 # GNU General Public License for more details.
13 13 #
14 14 # You should have received a copy of the GNU General Public License
15 15 # along with this program; if not, write to the Free Software
16 16 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
17 17
18 18 class IssueRelation < ActiveRecord::Base
19 19 belongs_to :issue_from, :class_name => 'Issue', :foreign_key => 'issue_from_id'
20 20 belongs_to :issue_to, :class_name => 'Issue', :foreign_key => 'issue_to_id'
21 21
22 22 TYPE_RELATES = "relates"
23 23 TYPE_DUPLICATES = "duplicates"
24 24 TYPE_DUPLICATED = "duplicated"
25 25 TYPE_BLOCKS = "blocks"
26 26 TYPE_BLOCKED = "blocked"
27 27 TYPE_PRECEDES = "precedes"
28 28 TYPE_FOLLOWS = "follows"
29 29
30 30 TYPES = { TYPE_RELATES => { :name => :label_relates_to, :sym_name => :label_relates_to, :order => 1, :sym => TYPE_RELATES },
31 31 TYPE_DUPLICATES => { :name => :label_duplicates, :sym_name => :label_duplicated_by, :order => 2, :sym => TYPE_DUPLICATED },
32 32 TYPE_DUPLICATED => { :name => :label_duplicated_by, :sym_name => :label_duplicates, :order => 3, :sym => TYPE_DUPLICATES, :reverse => TYPE_DUPLICATES },
33 33 TYPE_BLOCKS => { :name => :label_blocks, :sym_name => :label_blocked_by, :order => 4, :sym => TYPE_BLOCKED },
34 34 TYPE_BLOCKED => { :name => :label_blocked_by, :sym_name => :label_blocks, :order => 5, :sym => TYPE_BLOCKS, :reverse => TYPE_BLOCKS },
35 35 TYPE_PRECEDES => { :name => :label_precedes, :sym_name => :label_follows, :order => 6, :sym => TYPE_FOLLOWS },
36 36 TYPE_FOLLOWS => { :name => :label_follows, :sym_name => :label_precedes, :order => 7, :sym => TYPE_PRECEDES, :reverse => TYPE_PRECEDES }
37 37 }.freeze
38 38
39 39 validates_presence_of :issue_from, :issue_to, :relation_type
40 40 validates_inclusion_of :relation_type, :in => TYPES.keys
41 41 validates_numericality_of :delay, :allow_nil => true
42 42 validates_uniqueness_of :issue_to_id, :scope => :issue_from_id
43 43
44 44 attr_protected :issue_from_id, :issue_to_id
45 45
46 46 def validate
47 47 if issue_from && issue_to
48 48 errors.add :issue_to_id, :invalid if issue_from_id == issue_to_id
49 49 errors.add :issue_to_id, :not_same_project unless issue_from.project_id == issue_to.project_id || Setting.cross_project_issue_relations?
50 #detect circular dependencies depending wether the relation should be reversed
51 if TYPES.has_key?(relation_type) && TYPES[relation_type][:reverse]
52 errors.add_to_base :circular_dependency if issue_from.all_dependent_issues.include? issue_to
53 else
50 54 errors.add_to_base :circular_dependency if issue_to.all_dependent_issues.include? issue_from
55 end
51 56 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)
52 57 end
53 58 end
54 59
55 60 def other_issue(issue)
56 61 (self.issue_from_id == issue.id) ? issue_to : issue_from
57 62 end
58 63
59 64 # Returns the relation type for +issue+
60 65 def relation_type_for(issue)
61 66 if TYPES[relation_type]
62 67 if self.issue_from_id == issue.id
63 68 relation_type
64 69 else
65 70 TYPES[relation_type][:sym]
66 71 end
67 72 end
68 73 end
69 74
70 75 def label_for(issue)
71 76 TYPES[relation_type] ? TYPES[relation_type][(self.issue_from_id == issue.id) ? :name : :sym_name] : :unknow
72 77 end
73 78
74 79 def before_save
75 80 reverse_if_needed
76 81
77 82 if TYPE_PRECEDES == relation_type
78 83 self.delay ||= 0
79 84 else
80 85 self.delay = nil
81 86 end
82 87 set_issue_to_dates
83 88 end
84 89
85 90 def set_issue_to_dates
86 91 soonest_start = self.successor_soonest_start
87 92 if soonest_start && issue_to
88 93 issue_to.reschedule_after(soonest_start)
89 94 end
90 95 end
91 96
92 97 def successor_soonest_start
93 98 if (TYPE_PRECEDES == self.relation_type) && delay && issue_from && (issue_from.start_date || issue_from.due_date)
94 99 (issue_from.due_date || issue_from.start_date) + 1 + delay
95 100 end
96 101 end
97 102
98 103 def <=>(relation)
99 104 TYPES[self.relation_type][:order] <=> TYPES[relation.relation_type][:order]
100 105 end
101 106
102 107 private
103 108
104 109 # Reverses the relation if needed so that it gets stored in the proper way
105 110 def reverse_if_needed
106 111 if TYPES.has_key?(relation_type) && TYPES[relation_type][:reverse]
107 112 issue_tmp = issue_to
108 113 self.issue_to = issue_from
109 114 self.issue_from = issue_tmp
110 115 self.relation_type = TYPES[relation_type][:reverse]
111 116 end
112 117 end
113 118 end
Show file before | Show file after | Hide comments
@@ -1,100 +1,102
1 1 require File.expand_path('../../test_helper', __FILE__)
2 2 require 'issue_relations_controller'
3 3
4 4 # Re-raise errors caught by the controller.
5 5 class IssueRelationsController; def rescue_action(e) raise e end; end
6 6
7 7
8 8 class IssueRelationsControllerTest < ActionController::TestCase
9 9 fixtures :projects,
10 10 :users,
11 11 :roles,
12 12 :members,
13 13 :member_roles,
14 14 :issues,
15 15 :issue_statuses,
16 16 :issue_relations,
17 17 :enabled_modules,
18 18 :enumerations,
19 19 :trackers
20 20
21 21 def setup
22 22 @controller = IssueRelationsController.new
23 23 @request = ActionController::TestRequest.new
24 24 @response = ActionController::TestResponse.new
25 25 User.current = nil
26 26 end
27 27
28 28 def test_new
29 29 assert_difference 'IssueRelation.count' do
30 30 @request.session[:user_id] = 3
31 31 post :new, :issue_id => 1,
32 32 :relation => {:issue_to_id => '2', :relation_type => 'relates', :delay => ''}
33 33 end
34 34 end
35 35
36 36 def test_new_xhr
37 37 assert_difference 'IssueRelation.count' do
38 38 @request.session[:user_id] = 3
39 39 xhr :post, :new,
40 40 :issue_id => 3,
41 41 :relation => {:issue_to_id => '1', :relation_type => 'relates', :delay => ''}
42 42 assert_select_rjs 'relations' do
43 43 assert_select 'table', 1
44 44 assert_select 'tr', 2 # relations
45 45 end
46 46 end
47 47 end
48 48
49 49 def test_new_should_accept_id_with_hash
50 50 assert_difference 'IssueRelation.count' do
51 51 @request.session[:user_id] = 3
52 52 post :new, :issue_id => 1,
53 53 :relation => {:issue_to_id => '#2', :relation_type => 'relates', :delay => ''}
54 54 end
55 55 end
56 56
57 57 def test_new_should_not_break_with_non_numerical_id
58 58 assert_no_difference 'IssueRelation.count' do
59 59 assert_nothing_raised do
60 60 @request.session[:user_id] = 3
61 61 post :new, :issue_id => 1,
62 62 :relation => {:issue_to_id => 'foo', :relation_type => 'relates', :delay => ''}
63 63 end
64 64 end
65 65 end
66 66
67 67 def test_should_create_relations_with_visible_issues_only
68 68 Setting.cross_project_issue_relations = '1'
69 69 assert_nil Issue.visible(User.find(3)).find_by_id(4)
70 70
71 71 assert_no_difference 'IssueRelation.count' do
72 72 @request.session[:user_id] = 3
73 73 post :new, :issue_id => 1,
74 74 :relation => {:issue_to_id => '4', :relation_type => 'relates', :delay => ''}
75 75 end
76 76 end
77 77
78 should "prevent relation creation when there's a circular dependency"
79
78 80 def test_destroy
79 81 assert_difference 'IssueRelation.count', -1 do
80 82 @request.session[:user_id] = 3
81 83 post :destroy, :id => '2', :issue_id => '3'
82 84 end
83 85 end
84 86
85 87 def test_destroy_xhr
86 88 IssueRelation.create!(:relation_type => IssueRelation::TYPE_RELATES) do |r|
87 89 r.issue_from_id = 3
88 90 r.issue_to_id = 1
89 91 end
90 92
91 93 assert_difference 'IssueRelation.count', -1 do
92 94 @request.session[:user_id] = 3
93 95 xhr :post, :destroy, :id => '2', :issue_id => '3'
94 96 assert_select_rjs 'relations' do
95 97 assert_select 'table', 1
96 98 assert_select 'tr', 1 # relation left
97 99 end
98 100 end
99 101 end
100 102 end
Show file before | Show file after | Hide comments
@@ -1,85 +1,97
1 1 # Redmine - project management software
2 2 # Copyright (C) 2006-2009 Jean-Philippe Lang
3 3 #
4 4 # This program is free software; you can redistribute it and/or
5 5 # modify it under the terms of the GNU General Public License
6 6 # as published by the Free Software Foundation; either version 2
7 7 # of the License, or (at your option) any later version.
8 8 #
9 9 # This program is distributed in the hope that it will be useful,
10 10 # but WITHOUT ANY WARRANTY; without even the implied warranty of
11 11 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12 12 # GNU General Public License for more details.
13 13 #
14 14 # You should have received a copy of the GNU General Public License
15 15 # along with this program; if not, write to the Free Software
16 16 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
17 17
18 18 require File.expand_path('../../test_helper', __FILE__)
19 19
20 20 class IssueRelationTest < ActiveSupport::TestCase
21 21 fixtures :issue_relations, :issues
22 22
23 23 def test_create
24 24 from = Issue.find(1)
25 25 to = Issue.find(2)
26 26
27 27 relation = IssueRelation.new :issue_from => from, :issue_to => to, :relation_type => IssueRelation::TYPE_PRECEDES
28 28 assert relation.save
29 29 relation.reload
30 30 assert_equal IssueRelation::TYPE_PRECEDES, relation.relation_type
31 31 assert_equal from, relation.issue_from
32 32 assert_equal to, relation.issue_to
33 33 end
34 34
35 35 def test_follows_relation_should_be_reversed
36 36 from = Issue.find(1)
37 37 to = Issue.find(2)
38 38
39 39 relation = IssueRelation.new :issue_from => from, :issue_to => to, :relation_type => IssueRelation::TYPE_FOLLOWS
40 40 assert relation.save
41 41 relation.reload
42 42 assert_equal IssueRelation::TYPE_PRECEDES, relation.relation_type
43 43 assert_equal to, relation.issue_from
44 44 assert_equal from, relation.issue_to
45 45 end
46 46
47 # TODO : document why it shouldn't be reversed if validation fails : having
48 # relations reversed before the validation would allow simpler code for the
49 # validation
47 50 def test_follows_relation_should_not_be_reversed_if_validation_fails
48 51 from = Issue.find(1)
49 52 to = Issue.find(2)
50 53
51 54 relation = IssueRelation.new :issue_from => from, :issue_to => to, :relation_type => IssueRelation::TYPE_FOLLOWS, :delay => 'xx'
52 55 assert !relation.save
53 56 assert_equal IssueRelation::TYPE_FOLLOWS, relation.relation_type
54 57 assert_equal from, relation.issue_from
55 58 assert_equal to, relation.issue_to
56 59 end
57 60
58 61 def test_relation_type_for
59 62 from = Issue.find(1)
60 63 to = Issue.find(2)
61 64
62 65 relation = IssueRelation.new :issue_from => from, :issue_to => to, :relation_type => IssueRelation::TYPE_PRECEDES
63 66 assert_equal IssueRelation::TYPE_PRECEDES, relation.relation_type_for(from)
64 67 assert_equal IssueRelation::TYPE_FOLLOWS, relation.relation_type_for(to)
65 68 end
66 69
67 70 def test_set_issue_to_dates_without_issue_to
68 71 r = IssueRelation.new(:issue_from => Issue.new(:start_date => Date.today), :relation_type => IssueRelation::TYPE_PRECEDES, :delay => 1)
69 72 assert_nil r.set_issue_to_dates
70 73 end
71 74
72 75 def test_set_issue_to_dates_without_issues
73 76 r = IssueRelation.new(:relation_type => IssueRelation::TYPE_PRECEDES, :delay => 1)
74 77 assert_nil r.set_issue_to_dates
75 78 end
76 79
77 80 def test_validates_circular_dependency
78 81 IssueRelation.delete_all
79 82 assert IssueRelation.create!(:issue_from => Issue.find(1), :issue_to => Issue.find(2), :relation_type => IssueRelation::TYPE_PRECEDES)
80 83 assert IssueRelation.create!(:issue_from => Issue.find(2), :issue_to => Issue.find(3), :relation_type => IssueRelation::TYPE_PRECEDES)
81 84 r = IssueRelation.new(:issue_from => Issue.find(3), :issue_to => Issue.find(1), :relation_type => IssueRelation::TYPE_PRECEDES)
82 85 assert !r.save
83 86 assert_not_nil r.errors.on(:base)
84 87 end
88
89 def test_validates_circular_dependency_on_reverse_relations
90 IssueRelation.delete_all
91 assert IssueRelation.create!(:issue_from => Issue.find(1), :issue_to => Issue.find(3), :relation_type => IssueRelation::TYPE_BLOCKS)
92 assert IssueRelation.create!(:issue_from => Issue.find(1), :issue_to => Issue.find(2), :relation_type => IssueRelation::TYPE_BLOCKED)
93 r = IssueRelation.new(:issue_from => Issue.find(2), :issue_to => Issue.find(1), :relation_type => IssueRelation::TYPE_BLOCKED)
94 assert !r.save
95 assert_not_nil r.errors.on(:base)
96 end
85 97 end
General Comments 0
You need to be logged in to leave comments. Login now