Merge pull request #26 from finnlabs/fix/fixed_version_inheritance

Fix/fixed version inheritance

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|

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 }