[#32813] Cost reports should include work package children

frontend/src/app/shared/components/fields/display/field-types/wp-spent-time-display-field.module.ts

@@ -82,7 +82,7 @@ export class WorkPackageSpentTimeDisplayField extends WorkDisplayField {
             ),
           )
             .search(
-              `fields[]=WorkPackageId&operators[WorkPackageId]=%3D&values[WorkPackageId]=${wpID}&set_filter=1`,
+              `fields[]=WorkPackageId&operators[WorkPackageId]=%3D_child_work_packages&values[WorkPackageId]=${wpID}&set_filter=1`,
             )
             .toString();
 

modules/reporting/app/models/cost_query/filter/work_package_id.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 #-- copyright
 # OpenProject is an open source project management software.
 # Copyright (C) the OpenProject GmbH
@@ -31,6 +33,10 @@ def self.label
     WorkPackage.model_name.human
   end
 
+  def self.available_operators
+    ["=", "!", "=_child_work_packages", "!_child_work_packages"].map(&:to_operator)
+  end
+
   def self.available_values(*)
     WorkPackage
       .where(project_id: Project.allowed_to(User.current, :view_work_packages))
@@ -39,27 +45,28 @@ def self.available_values(*)
       .map { |id, subject| [text_for_tuple(id, subject), id] }
   end
 
-  def self.available_operators
-    ["="].map(&:to_operator)
-  end
-
   ##
   # Overwrites Report::Filter::Base self.label_for_value method
   # to achieve a more performant implementation
   def self.label_for_value(value)
     return nil unless value.to_i.to_s == value.to_s # we expect an work_package-id
 
     work_package = WorkPackage.find(value.to_i)
-    [text_for_work_package(work_package), work_package.id] if work_package and work_package.visible?(User.current)
+    [text_for_work_package(work_package), work_package.id] if work_package&.visible?(User.current)
   end
 
   def self.text_for_tuple(id, subject)
     str = "##{id} "
-    str << (subject.length > 30 ? subject.first(26) + "..." : subject)
+    str << (subject.length > 30 ? "#{subject.first(26)}..." : subject)
   end
 
-  def self.text_for_work_package(i)
-    i = i.first if i.is_a? Array
-    text_for_touble(i.id, i.subject)
+  def self.text_for_work_package(work_package_or_work_package_list)
+    wp = if work_package_or_work_package_list.is_a?(Array)
+           work_package_or_work_package_list.first
+         else
+           work_package_or_work_package_list
+         end
+
+    text_for_tuple(wp.id, wp.subject)
   end
 end

modules/reporting/app/models/cost_query/operator.rb

@@ -50,8 +50,13 @@ def modify(query, field, *_values)
     def modify(query, field, *values)
       p_ids = []
       values.each do |value|
-        p_ids += ([value] << Project.find(value).descendants.map(&:id))
+        project = Project.visible.find(value)
+        next unless project
+
+        p_ids << project.id
+        p_ids += project.descendants.pluck(:id)
       end
+
       "=".to_operator.modify query, field, p_ids
     rescue ActiveRecord::RecordNotFound
       query
@@ -63,12 +68,53 @@ def modify(query, field, *values)
       p_ids = []
       values.each do |value|
         value.to_s.split(",").each do |id|
-          p_ids += ([id] << Project.find(id).descendants.map(&:id))
+          project = Project.visible.find(id)
+          next unless project
+
+          p_ids << project.id
+          p_ids += project.descendants.pluck(:id)
         end
       end
+
       "!".to_operator.modify query, field, p_ids
     rescue ActiveRecord::RecordNotFound
       query
     end
   end
+
+  new "=_child_work_packages", validate: :integers, label: :label_is_work_package_with_descendants do
+    def modify(query, field, *values)
+      wp_ids = []
+      values.each do |value|
+        work_package = WorkPackage.visible.find(value)
+        next unless work_package
+
+        wp_ids << work_package.id
+        wp_ids += work_package.descendants.pluck(:id)
+      end
+
+      "=".to_operator.modify query, field, wp_ids
+    rescue ActiveRecord::RecordNotFound
+      query
+    end
+  end
+
+  new "!_child_work_packages", validate: :integers, label: :label_is_not_work_package_with_descendants do
+    def modify(query, field, *values)
+      wp_ids = []
+      values.each do |value|
+        value.to_s.split(",").each do |id|
+          work_package = WorkPackage.visible.find(id)
+          next unless work_package
+
+          wp_ids << work_package.id
+          wp_ids += work_package.descendants.pluck(:id)
+        end
+      end
+
+      "!".to_operator.modify query, field, wp_ids
+    rescue ActiveRecord::RecordNotFound
+      query
+    end
+  end
 end

modules/reporting/config/locales/en.yml

@@ -58,6 +58,8 @@ en:
   label_greater: ">"
   label_is_not_project_with_subprojects: "is not (includes subprojects)"
   label_is_project_with_subprojects: "is (includes subprojects)"
+  label_is_work_package_with_descendants: "is (includes descendants)"
+  label_is_not_work_package_with_descendants: "is not (includes descendants)"
   label_work_package_attributes: "Work package attributes"
   label_less: "<"
   label_logged_by_reporting: "Logged by"

modules/reporting/spec/models/cost_query/filter_spec.rb

@@ -101,7 +101,7 @@ def create_work_package_with_time_entry(work_package_params = {}, entry_params =
                  activity:)
         end
 
-        it "onlies return entries from the given #{filter}" do
+        it "only return entries from the given #{filter}" do
           query.filter field, value: object.id
           query.result.each do |result|
             expect(result[field].to_s).to eq(object.id.to_s)
@@ -156,7 +156,7 @@ def create_work_package_with_time_entry(work_package_params = {}, entry_params =
                activity:)
       end
 
-      it "onlies return entries from the given CostQuery::Filter::AuthorId" do
+      it "only return entries from the given CostQuery::Filter::AuthorId" do
         query.filter "author_id", value: author.id
         query.result.each do |result|
           work_package_id = result["work_package_id"]
@@ -312,7 +312,7 @@ def create_matching_object_with_time_entries(factory, work_package_field, entry_
       CostQuery::Filter::PriorityId,
       CostQuery::Filter::TypeId
     ].each do |filter|
-      it "onlies allow default operators for #{filter}" do
+      it "only allow default operators for #{filter}" do
         expect(filter.new.available_operators.uniq.sort).to eq(CostQuery::Operator.default_operators.uniq.sort)
       end
     end
@@ -323,7 +323,7 @@ def create_matching_object_with_time_entries(factory, work_package_field, entry_
       CostQuery::Filter::CategoryId,
       CostQuery::Filter::VersionId
     ].each do |filter|
-      it "onlies allow default+null operators for #{filter}" do
+      it "only allow default+null operators for #{filter}" do
         expect(filter.new.available_operators.uniq.sort).to eq((CostQuery::Operator.default_operators + CostQuery::Operator.null_operators).sort)
       end
     end
@@ -332,8 +332,13 @@ def create_matching_object_with_time_entries(factory, work_package_field, entry_
     [
       CostQuery::Filter::WorkPackageId
     ].each do |filter|
-      it "onlies allow default operators for #{filter}" do
-        expect(filter.new.available_operators.uniq).to contain_exactly(CostQuery::Operator.default_operator)
+      it "allows custom filters#{filter}" do
+        expect(filter.new.available_operators.uniq).to contain_exactly(
+          Report::Operator.new("!"),
+          CostQuery::Operator.new("!_child_work_packages"),
+          Report::Operator.new("="),
+          CostQuery::Operator.new("=_child_work_packages")
+        )
       end
     end
 
@@ -345,7 +350,7 @@ def create_matching_object_with_time_entries(factory, work_package_field, entry_
       CostQuery::Filter::StartDate,
       CostQuery::Filter::DueDate
     ].each do |filter|
-      it "onlies allow time operators for #{filter}" do
+      it "only allow time operators for #{filter}" do
         expect(filter.new.available_operators.uniq.sort).to eq(CostQuery::Operator.time_operators.sort)
       end
     end