From 8a9123de04569aaaf0f8a337eaa8dbfb7fddd005 Mon Sep 17 00:00:00 2001
From: Go MAEDA <maeda@farend.jp>
Date: Wed, 7 Aug 2019 04:44:19 +0000
Subject: [PATCH] Limit total_estimated_hours to visible issues (#31778).

Patch by Gregor Schmidt.


git-svn-id: http://svn.redmine.org/redmine/trunk@18356 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/models/issue.rb                |  2 +-
 app/models/issue_query.rb          |  4 ++--
 test/unit/issue_subtasking_test.rb |  5 ++++-
 test/unit/query_test.rb            | 33 ++++++++++++++++++++++++++++++
 4 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 0e5ac530a..507574de7 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -1077,7 +1077,7 @@ class Issue < ActiveRecord::Base
     if leaf?
       estimated_hours
     else
-      @total_estimated_hours ||= self_and_descendants.sum(:estimated_hours)
+      @total_estimated_hours ||= self_and_descendants.visible.sum(:estimated_hours)
     end
   end
 
diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb
index 589089bad..5245d5ba0 100644
--- a/app/models/issue_query.rb
+++ b/app/models/issue_query.rb
@@ -40,8 +40,8 @@ class IssueQuery < Query
     QueryColumn.new(:due_date, :sortable => "#{Issue.table_name}.due_date", :groupable => true),
     QueryColumn.new(:estimated_hours, :sortable => "#{Issue.table_name}.estimated_hours", :totalable => true),
     QueryColumn.new(:total_estimated_hours,
-      :sortable => "COALESCE((SELECT SUM(estimated_hours) FROM #{Issue.table_name} subtasks" +
-        " WHERE subtasks.root_id = #{Issue.table_name}.root_id AND subtasks.lft >= #{Issue.table_name}.lft AND subtasks.rgt <= #{Issue.table_name}.rgt), 0)",
+      :sortable => -> { "COALESCE((SELECT SUM(estimated_hours) FROM #{Issue.table_name} subtasks" +
+        " WHERE #{Issue.visible_condition(User.current).gsub(/\bissues\b/, 'subtasks')} AND subtasks.root_id = #{Issue.table_name}.root_id AND subtasks.lft >= #{Issue.table_name}.lft AND subtasks.rgt <= #{Issue.table_name}.rgt), 0)" },
       :default_order => 'desc'),
     QueryColumn.new(:done_ratio, :sortable => "#{Issue.table_name}.done_ratio", :groupable => true),
     TimestampQueryColumn.new(:created_on, :sortable => "#{Issue.table_name}.created_on", :default_order => 'desc', :groupable => true),
diff --git a/test/unit/issue_subtasking_test.rb b/test/unit/issue_subtasking_test.rb
index f0a13e5f2..51d30d7e9 100644
--- a/test/unit/issue_subtasking_test.rb
+++ b/test/unit/issue_subtasking_test.rb
@@ -338,7 +338,7 @@ class IssueSubtaskingTest < ActiveSupport::TestCase
     end
   end
 
-  def test_parent_total_estimated_hours_should_be_sum_of_descendants
+  def test_parent_total_estimated_hours_should_be_sum_of_visible_descendants
     parent = Issue.generate!
     parent.generate_child!(:estimated_hours => nil)
     assert_equal 0, parent.reload.total_estimated_hours
@@ -346,6 +346,9 @@ class IssueSubtaskingTest < ActiveSupport::TestCase
     assert_equal 5, parent.reload.total_estimated_hours
     parent.generate_child!(:estimated_hours => 7)
     assert_equal 12, parent.reload.total_estimated_hours
+
+    parent.generate_child!(:estimated_hours => 9, :is_private => true)
+    assert_equal 12, parent.reload.total_estimated_hours
   end
 
   def test_open_issue_with_closed_parent_should_not_validate
diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb
index 4e590b759..2a2a2e99c 100644
--- a/test/unit/query_test.rb
+++ b/test/unit/query_test.rb
@@ -1624,6 +1624,39 @@ class QueryTest < ActiveSupport::TestCase
     assert_equal values.sort, values
   end
 
+  def test_sort_by_total_for_estimated_hours
+    # Prepare issues
+    parent = issues(:issues_001)
+    child = issues(:issues_002)
+    private_child = issues(:issues_003)
+    other = issues(:issues_007)
+
+    User.current = users(:users_001)
+
+    parent.safe_attributes         = {:estimated_hours => 1}
+    child.safe_attributes          = {:estimated_hours => 2, :parent_issue_id => 1}
+    private_child.safe_attributes  = {:estimated_hours => 4, :parent_issue_id => 1, :is_private => true}
+    other.safe_attributes          = {:estimated_hours => 5}
+
+    [parent, child, private_child, other].each(&:save!)
+
+
+    q = IssueQuery.new(
+      :name => '_',
+      :filters => { 'issue_id' => {:operator => '=', :values => ['1,7']} },
+      :sort_criteria => [['total_estimated_hours', 'asc']]
+    )
+
+    # With private_child, `parent' is "bigger" than `other'
+    ids = q.issue_ids
+    assert_equal [7, 1], ids, "Private issue was not used to calculate sort order"
+
+    # Without the invisible private_child, `other' is "bigger" than `parent'
+    User.current = User.anonymous
+    ids = q.issue_ids
+    assert_equal [1, 7], ids, "Private issue was used to calculate sort order"
+  end
+
   def test_set_totalable_names
     q = IssueQuery.new
     q.totalable_names = ['estimated_hours', :spent_hours, '']
-- 
2.39.5