From 1f12393880265481bb453565562c7bf11d5be9fb Mon Sep 17 00:00:00 2001 From: Aaron Contreras Date: Tue, 24 Sep 2024 16:56:36 -0500 Subject: [PATCH] Account for status exclusion from total calculations in simple average mode --- .../work_packages/update_ancestors_service.rb | 13 +++-- ..._total_percent_complete_mode_change_job.rb | 23 ++++++-- .../work_packages/progress/sql_commands.rb | 7 ++- .../update_ancestors_service_spec.rb | 26 +++++++++ ...l_percent_complete_mode_change_job_spec.rb | 55 +++++++++++++++++++ 5 files changed, 111 insertions(+), 13 deletions(-) diff --git a/app/services/work_packages/update_ancestors_service.rb b/app/services/work_packages/update_ancestors_service.rb index 7e44b2387408..e70189edeafe 100644 --- a/app/services/work_packages/update_ancestors_service.rb +++ b/app/services/work_packages/update_ancestors_service.rb @@ -140,11 +140,9 @@ def calculate_work_weighted_average_percent_complete(work_package) end def calculate_simple_average_percent_complete(work_package, loader) - all_done_ratios = loader - .children_of(work_package) - .map { |child| child.derived_done_ratio || child.done_ratio || 0 } + all_done_ratios = children_done_ratio_values(work_package, loader) - if work_package.done_ratio.present? + if work_package.done_ratio.present? && !work_package.status.excluded_from_totals all_done_ratios << work_package.done_ratio end @@ -152,6 +150,13 @@ def calculate_simple_average_percent_complete(work_package, loader) progress.round end + def children_done_ratio_values(work_package, loader) + loader + .children_of(work_package) + .reject(&:status_excluded_from_totals) + .map { |child| child.derived_done_ratio || child.done_ratio || 0 } + end + # Sets the ignore_non_working_days to true if any descendant has its value set to true. # If there is no value returned from the descendants, that means that the work package in # question no longer has a descendant. But since we are in the service going up the ancestor chain, diff --git a/app/workers/work_packages/progress/apply_total_percent_complete_mode_change_job.rb b/app/workers/work_packages/progress/apply_total_percent_complete_mode_change_job.rb index 7feb4fc7e2c8..a2c614497d6c 100644 --- a/app/workers/work_packages/progress/apply_total_percent_complete_mode_change_job.rb +++ b/app/workers/work_packages/progress/apply_total_percent_complete_mode_change_job.rb @@ -106,7 +106,12 @@ def update_to_simple_average WHEN current_depth = min_depth THEN NULL ELSE ROUND( ( - COALESCE(wp.p_complete, 0) + ( + /* Exclude the current work package if it has a status excluded from totals */ + CASE WHEN wp.status_excluded_from_totals + THEN 0 + /* Otherwise, use the current work package's % complete value or 0 if unset */ + ELSE COALESCE(wp.p_complete, 0) + END + ( SELECT SUM( COALESCE(child_wp.total_p_complete, child_wp.p_complete, 0) @@ -115,18 +120,24 @@ def update_to_simple_average temp_wp_progress_values child_wp WHERE child_wp.parent_id = wp.id + /* Exclude children with a status excluded from totals */ + AND NOT child_wp.status_excluded_from_totals ) - ) / ( - CASE - WHEN wp.p_complete IS NOT NULL THEN 1 - ELSE 0 - END + ( + ) / ( + /* Exclude the current work package if it has a status excluded from totals */ + CASE WHEN wp.status_excluded_from_totals + THEN 0 + /* Otherwise, count the current work package if it has a % complete value set */ + ELSE(CASE WHEN wp.p_complete IS NOT NULL THEN 1 ELSE 0 END) + END + ( SELECT COUNT(1) FROM temp_wp_progress_values child_wp WHERE child_wp.parent_id = wp.id + /* Exclude children with a status excluded from totals */ + AND NOT child_wp.status_excluded_from_totals ) ) ) diff --git a/app/workers/work_packages/progress/sql_commands.rb b/app/workers/work_packages/progress/sql_commands.rb index ce4ff402aea0..4f2c77771202 100644 --- a/app/workers/work_packages/progress/sql_commands.rb +++ b/app/workers/work_packages/progress/sql_commands.rb @@ -95,12 +95,13 @@ def create_temporary_total_percent_complete_table_for_simple_average_mode execute(<<~SQL.squish) CREATE UNLOGGED TABLE temp_wp_progress_values AS SELECT - id, - parent_id, - status_id, + work_packages.id as id, + work_packages.parent_id as parent_id, + statuses.excluded_from_totals AS status_excluded_from_totals, done_ratio AS p_complete, NULL::INTEGER AS total_p_complete FROM work_packages + LEFT JOIN statuses ON work_packages.status_id = statuses.id SQL end diff --git a/spec/services/work_packages/update_ancestors_service_spec.rb b/spec/services/work_packages/update_ancestors_service_spec.rb index 57e5cfeddeae..dac34b0aabcc 100644 --- a/spec/services/work_packages/update_ancestors_service_spec.rb +++ b/spec/services/work_packages/update_ancestors_service_spec.rb @@ -975,6 +975,32 @@ def call_update_ancestors_service(work_package) TABLE end end + + context "with parent having % complete set " \ + "and a multi-level children hierarchy with some % complete set " \ + "and some children having status excluded from totals" do + let_work_packages(<<~TABLE) + hierarchy | status | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | Open | | 44h | | 21h | 10% | + child1 | Open | 15h | 23h | 10h | 14h | 33% | 43% + grandchild1 | Open | 5h | | 3h | | 40% | + grandchild2 | Open | 3h | | 1h | | 67% | + child2 | Rejected | 5h | 7h | 2.5h | 3.5h | 50% | 75% + grandchild3 | Open | 2h | | 1h | | 100% | + child3 | Open | 10h | 14h | 2.5h | 3.5h | 75% | + grandchild4 | Rejected | 4h | | 1h | | 75% | + child4 | Open | | | | | 60% | + TABLE + + it "sets the total % complete solely based on % complete values of direct children" do + expect(call_result).to be_success + updated_work_packages = call_result.all_results + expect_work_packages(updated_work_packages, <<~TABLE) + subject | total % complete + parent | 47% + TABLE + end + end end describe "ignore_non_working_days propagation" do diff --git a/spec/workers/work_packages/progress/apply_total_percent_complete_mode_change_job_spec.rb b/spec/workers/work_packages/progress/apply_total_percent_complete_mode_change_job_spec.rb index 4ad6cae47099..5e393754f545 100644 --- a/spec/workers/work_packages/progress/apply_total_percent_complete_mode_change_job_spec.rb +++ b/spec/workers/work_packages/progress/apply_total_percent_complete_mode_change_job_spec.rb @@ -185,6 +185,33 @@ def expect_performing_job_changes(from:, to:, end end + context "on a multi-level hierarchy with work and remaining work values set " \ + "and a child with a status excluded from totals" do + it "updates the total % complete of parent work packages excluding the child" do + expect_performing_job_changes( + mode: "work_weighted_average", + from: <<~TABLE, + hierarchy | status | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | New | 10h | 30h | 6h | 6h | 40% | 63% + child1 | New | 15h | | 0h | | 100% | + child2 | New | | | | | 40% | + child3 | New | 5h | 5h | 0h | 0h | 100% | 70% + grandchild1 | New | | | | | 40% | + grandchild2 | Excluded | 20h | | 0h | | 100% | + TABLE + to: <<~TABLE + subject | status | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | New | 10h | 30h | 6h | 6h | 40% | 80% + child1 | New | 15h | | 0h | | 100% | + child2 | New | | | | | 40% | + child3 | New | 5h | 5h | 0h | 0h | 100% | 100% + grandchild1 | New | | | | | 40% | + grandchild2 | Excluded | 20h | | 0h | | 100% | + TABLE + ) + end + end + describe "journal entries" do # rubocop:disable RSpec/ExampleLength it "creates journal entries for the modified work packages" do @@ -341,6 +368,34 @@ def expect_performing_job_changes(from:, to:, end end + context "on a multi-level hierarchy with work and remaining work values set " \ + "and a child having a status excluded from totals" do + it "updates the total % complete of parent work packages irrelevant of work values " \ + "excluding the child with the excluded status" do + expect_performing_job_changes( + mode: "simple_average", + from: <<~TABLE, + hierarchy | status | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | New | 10h | 50h | 6h | 6h | 40% | 88% + child1 | New | 15h | | 0h | | 100% | + child2 | New | | | | | 40% | + child3 | Excluded | 5h | 25h | 0h | 0h | 100% | 100% + grandchild1 | New | | | | | 40% | + grandchild2 | New | 20h | | 0h | | 100% | + TABLE + to: <<~TABLE + subject | status | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | New | 10h | 50h | 6h | 6h | 40% | 60% + child1 | New | 15h | | 0h | | 100% | + child2 | New | | | | | 40% | + child3 | Excluded | 5h | 25h | 0h | 0h | 100% | 70% + grandchild1 | New | | | | | 40% | + grandchild2 | New | 20h | | 0h | | 100% | + TABLE + ) + end + end + describe "journal entries" do # rubocop:disable RSpec/ExampleLength it "creates journal entries for the modified work packages" do