diff --git a/app/models/issue.rb b/app/models/issue.rb index b4113b9fe..39b4c2dbf 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -1243,12 +1243,26 @@ class Issue < ApplicationRecord # Preloads visible total spent time for a collection of issues def self.load_visible_total_spent_hours(issues, user=User.current) - if issues.any? + return if issues.empty? + + # For leaf issues, `total_spent_hours` equals `spent_hours`, so reuse + # the preloaded visible spent hours to avoid the expensive query below. + optimizable_leaf_issues, remaining_issues = issues.partition do |issue| + issue.leaf? && + issue.instance_variable_defined?(:@spent_hours) && + !issue.instance_variable_get(:@spent_hours).nil? + end + + optimizable_leaf_issues.each do |issue| + issue.instance_variable_set :@total_spent_hours, issue.spent_hours + end + + if remaining_issues.any? hours_by_issue_id = TimeEntry.visible(user).joins(:issue). joins("JOIN #{Issue.table_name} parent ON parent.root_id = #{Issue.table_name}.root_id" + " AND parent.lft <= #{Issue.table_name}.lft AND parent.rgt >= #{Issue.table_name}.rgt"). - where("parent.id IN (?)", issues.map(&:id)).group("parent.id").sum(:hours) - issues.each do |issue| + where("parent.id IN (?)", remaining_issues.map(&:id)).group("parent.id").sum(:hours) + remaining_issues.each do |issue| issue.instance_variable_set :@total_spent_hours, (hours_by_issue_id[issue.id] || 0.0) end end diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index a5c0bbb87..77c2a3a91 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -222,6 +222,18 @@ class IssueTest < ActiveSupport::TestCase assert_include 'Parent task is invalid', issue.errors.full_messages end + def test_load_visible_total_spent_hours_should_use_loaded_spent_hours_for_leaf_issues + issue = Issue.find(1) + assert issue.leaf? + # preloaded visible spent hours value is stored in @spent_hours instance variable + issue.instance_variable_set(:@spent_hours, 3.5) + TimeEntry.expects(:visible).never + + Issue.load_visible_total_spent_hours([issue]) + + assert_equal 3.5, issue.instance_variable_get(:@total_spent_hours) + end + def assert_visibility_match(user, issues) assert_equal issues.collect(&:id).sort, Issue.all.select {|issue| issue.visible?(user)}.collect(&:id).sort end