diff --git a/lib/open_project/backlogs/patches/issue_patch.rb b/lib/open_project/backlogs/patches/issue_patch.rb index 0eff7a23be..c138b137cf 100644 --- a/lib/open_project/backlogs/patches/issue_patch.rb +++ b/lib/open_project/backlogs/patches/issue_patch.rb @@ -9,11 +9,11 @@ module OpenProject::Backlogs::Patches::IssuePatch extend ClassMethods alias_method_chain :recalculate_attributes_for, :remaining_hours - before_validation :backlogs_before_validation, :if => lambda {|i| i.project && i.project.module_enabled?("backlogs")} + before_validation :backlogs_before_validation, :if => lambda {|i| i.project && i.backlogs_enabled?} - before_save :inherit_version_from_closest_story_or_impediment, :if => lambda {|i| i.is_task? } + before_save :inherit_version_from_closest_story_or_impediment, :if => lambda {|i| i.is_task? && i.backlogs_enabled? } after_save :inherit_version_to_descendants, :if => lambda {|i| (i.fixed_version_id_changed? && i.backlogs_enabled? && i.closest_story_or_impediment == i) } - after_move :inherit_version_to_descendants, :if => lambda {|i| (i.is_task?) } + after_move :inherit_version_to_descendants, :if => lambda {|i| (i.is_task? && i.backlogs_enabled?) } register_on_journal_formatter(:fraction, 'remaining_hours') register_on_journal_formatter(:decimal, 'story_points') @@ -97,7 +97,7 @@ module OpenProject::Backlogs::Patches::IssuePatch end def is_story? - backlogs_enabled? and Story.trackers.include?(self.tracker_id) + backlogs_enabled? && Story.trackers.include?(self.tracker_id) end def to_task @@ -105,11 +105,11 @@ module OpenProject::Backlogs::Patches::IssuePatch end def is_task? - backlogs_enabled? and (self.parent_issue_id && self.tracker_id == Task.tracker && Task.tracker.present?) + backlogs_enabled? && (self.parent_issue_id && self.tracker_id == Task.tracker && Task.tracker.present?) end def is_impediment? - backlogs_enabled? and (self.parent_issue_id.nil? && self.tracker_id == Task.tracker && Task.tracker.present?) + backlogs_enabled? && (self.parent_issue_id.nil? && self.tracker_id == Task.tracker && Task.tracker.present?) end def trackers @@ -171,31 +171,43 @@ module OpenProject::Backlogs::Patches::IssuePatch end def backlogs_enabled? - self.project.try(:module_enabled?, "backlogs") + !!self.project.try(:module_enabled?, "backlogs") end def in_backlogs_tracker? - backlogs_enabled? and Issue.backlogs_trackers.include?(self.tracker.id) + backlogs_enabled? && Issue.backlogs_trackers.include?(self.tracker.id) end - def closest_story_or_impediment - return nil unless in_backlogs_tracker? - return self if self.is_story? - - root = self + # ancestors array similar to Module#ancestors + # i.e. returns immediate ancestors first + def ancestor_chain + ancestors = [] unless self.parent_issue_id.nil? - real_parent = Issue.find_by_id(self.parent_issue_id) # Unfortunately the nested set is only build on save hence, the #parent # method is not always correct. Therefore we go to the parent the hard # way and use nested set from there - ancestors = real_parent.ancestors.find_all_by_tracker_id(Issue.backlogs_trackers) - ancestors ? ancestors << real_parent : [real_parent] + real_parent = Issue.find_by_id(self.parent_issue_id) - root = ancestors.sort_by(&:right).find { |i| i.is_story? or i.is_impediment? } + # Sort immediate ancestors first + ancestors = ([real_parent] + real_parent.ancestors).sort_by(&:right) end + ancestors + end - root + def closest_story_or_impediment + return nil unless in_backlogs_tracker? + return self if (self.is_story? || self.is_impediment?) + closest = nil + ancestor_chain.each do |i| + # break if we found an element in our chain that is not relevant in backlogs + break unless i.in_backlogs_tracker? + if (i.is_story? || i.is_impediment?) && (i.ancestor_chain.all? {|a| a.in_backlogs_tracker?}) + closest = i + break + end + end + closest end private @@ -221,7 +233,7 @@ module OpenProject::Backlogs::Patches::IssuePatch # way, the fixed_version_id is propagated up by the # inherit_version_from_closest_story_or_impediment before_filter and # the update_parent_attributes after_filter - stop_descendants, descendant_tasks = self.descendants.partition{|d| d.tracker_id != Task.tracker } + descendant_tasks, stop_descendants = self.descendants.partition { |d| d.is_task? } descendant_tasks.reject!{ |t| stop_descendants.any? { |s| s.left < t.left && s.right > t.right } } descendant_tasks.each do |task| diff --git a/spec/models/issue_fixed_version_propagated_down_spec.rb b/spec/models/issue_fixed_version_propagated_down_spec.rb index 659a2b39d6..c60a40dece 100644 --- a/spec/models/issue_fixed_version_propagated_down_spec.rb +++ b/spec/models/issue_fixed_version_propagated_down_spec.rb @@ -33,7 +33,6 @@ describe Issue, "changing a story's fixed_version changes the fixed_version of a :status => issue_status, :author => user, :priority => issue_priority) - story.project.enabled_module_names += ["backlogs"] story end @@ -46,7 +45,6 @@ describe Issue, "changing a story's fixed_version changes the fixed_version of a :status => issue_status, :author => user, :priority => issue_priority) - story.project.enabled_module_names += ["backlogs"] story end @@ -59,7 +57,6 @@ describe Issue, "changing a story's fixed_version changes the fixed_version of a :status => issue_status, :author => user, :priority => issue_priority) - story.project.enabled_module_names += ["backlogs"] story end @@ -237,6 +234,9 @@ describe Issue, "changing a story's fixed_version changes the fixed_version of a end describe "WITH backlogs enabled" do + before(:each) do + project.enabled_module_names += ["backlogs"] + end describe "WITH a story" do subject { story } @@ -434,6 +434,10 @@ describe Issue, "changing a story's fixed_version changes the fixed_version of a end describe "WITH backogs enabled" do + before(:each) do + story.project.enabled_module_names += ["backlogs"] + end + describe "WITH a story as parent" do let(:parent) { story }