From 2f2561c08ff2587bbe6cb72ec497e2dc9e7803d3 Mon Sep 17 00:00:00 2001 From: Aaron Contreras Date: Mon, 23 Sep 2024 17:54:23 -0500 Subject: [PATCH] Add in simple average calculation mode job * Adds temporary depth table - Could be cleaned up down thie line with the `work_package_hierarchies` table. --- ...complete_mode_changed_to_simple_average.rb | 33 ++++ app/services/settings/update_service.rb | 4 + ..._total_percent_complete_mode_change_job.rb | 79 ++++++--- .../work_packages/progress/sql_commands.rb | 72 +++++++- config/locales/en.yml | 5 +- lib/open_project/journal_formatter/cause.rb | 6 + .../progress_tracking_controller_spec.rb | 25 ++- ...l_percent_complete_mode_change_job_spec.rb | 163 +++++++++++++++++- 8 files changed, 352 insertions(+), 35 deletions(-) create mode 100644 app/models/journal/caused_by_total_percent_complete_mode_changed_to_simple_average.rb diff --git a/app/models/journal/caused_by_total_percent_complete_mode_changed_to_simple_average.rb b/app/models/journal/caused_by_total_percent_complete_mode_changed_to_simple_average.rb new file mode 100644 index 000000000000..2adcf77163df --- /dev/null +++ b/app/models/journal/caused_by_total_percent_complete_mode_changed_to_simple_average.rb @@ -0,0 +1,33 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +class Journal::CausedByTotalPercentCompleteModeChangedToSimpleAverage < CauseOfChange::Base + def initialize + super("total_percent_complete_mode_changed_to_simple_average") + end +end diff --git a/app/services/settings/update_service.rb b/app/services/settings/update_service.rb index 0cab0e99ceb8..0744628f29fa 100644 --- a/app/services/settings/update_service.rb +++ b/app/services/settings/update_service.rb @@ -51,6 +51,10 @@ def set_setting_value(name, value) WorkPackages::Progress::ApplyTotalPercentCompleteModeChangeJob .perform_later(mode: new_value, cause_type: "total_percent_complete_mode_changed_to_work_weighted_average") + elsif name == :total_percent_complete_mode && old_value != "simple_average" && new_value == "simple_average" + WorkPackages::Progress::ApplyTotalPercentCompleteModeChangeJob + .perform_later(mode: new_value, + cause_type: "total_percent_complete_mode_changed_to_simple_average") end end 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 b97b1ed1701a..7feb4fc7e2c8 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 @@ -92,30 +92,61 @@ def update_to_work_weighted_average def update_to_simple_average execute(<<~SQL.squish) - UPDATE temp_wp_progress_values - SET derived_done_ratio = CASE - WHEN avg_ratios.avg_done_ratio IS NOT NULL THEN ROUND(avg_ratios.avg_done_ratio) - ELSE done_ratio - END - FROM ( - SELECT wp_tree.ancestor_id AS id, - AVG(CASE - WHEN wp_tree.generations = 1 THEN COALESCE(wp_progress.done_ratio, 0) - ELSE NULL - END) AS avg_done_ratio - FROM work_package_hierarchies wp_tree - LEFT JOIN temp_wp_progress_values wp_progress ON wp_tree.descendant_id = wp_progress.id - LEFT JOIN statuses ON wp_progress.status_id = statuses.id - WHERE statuses.excluded_from_totals = FALSE - GROUP BY wp_tree.ancestor_id - ) avg_ratios - WHERE temp_wp_progress_values.id = avg_ratios.id - AND temp_wp_progress_values.id IN ( - SELECT ancestor_id AS id - FROM work_package_hierarchies - GROUP BY id - HAVING MAX(generations) > 0 - ) + DO $$ + DECLARE + min_depth INTEGER := 0; + max_depth INTEGER := (SELECT MAX(depth) FROM temp_work_package_depth); + current_depth INTEGER := min_depth; + BEGIN + /* Navigate work packages and perform updates bottom-up */ + while current_depth <= max_depth loop + UPDATE temp_wp_progress_values wp + SET + total_p_complete = CASE + WHEN current_depth = min_depth THEN NULL + ELSE ROUND( + ( + COALESCE(wp.p_complete, 0) + ( + SELECT + SUM( + COALESCE(child_wp.total_p_complete, child_wp.p_complete, 0) + ) + FROM + temp_wp_progress_values child_wp + WHERE + child_wp.parent_id = wp.id + ) + ) / ( + CASE + WHEN wp.p_complete IS NOT NULL THEN 1 + ELSE 0 + END + ( + SELECT + COUNT(1) + FROM + temp_wp_progress_values child_wp + WHERE + child_wp.parent_id = wp.id + ) + ) + ) + END + /* Select only work packages at the curren depth */ + WHERE + wp.id IN ( + SELECT + id + FROM + temp_work_package_depth + WHERE + depth = current_depth + ); + + /* Go up a level from a child to a parent*/ + current_depth := current_depth + 1; + + END loop; + END $$; SQL end diff --git a/app/workers/work_packages/progress/sql_commands.rb b/app/workers/work_packages/progress/sql_commands.rb index c142597637df..ce4ff402aea0 100644 --- a/app/workers/work_packages/progress/sql_commands.rb +++ b/app/workers/work_packages/progress/sql_commands.rb @@ -67,6 +67,7 @@ def with_temporary_total_percent_complete_table create_temporary_total_percent_complete_table_for_work_weighted_average_mode when "simple_average" create_temporary_total_percent_complete_table_for_simple_average_mode + create_temporary_depth_table else raise ArgumentError, "Invalid total percent complete mode: #{mode}" end @@ -74,24 +75,86 @@ def with_temporary_total_percent_complete_table yield ensure drop_temporary_total_percent_complete_table + drop_temporary_depth_table end end def create_temporary_total_percent_complete_table_for_work_weighted_average_mode execute(<<~SQL.squish) - CREATE UNLOGGED TABLE temp_wp_progress_values - AS SELECT + CREATE UNLOGGED TABLE temp_wp_progress_values AS + SELECT id, derived_estimated_hours as total_work, derived_remaining_hours as total_remaining_work, - derived_done_ratio as total_p_complete + derived_done_ratio as total_p_complete + FROM work_packages + SQL + end + + 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, + done_ratio AS p_complete, + NULL::INTEGER AS total_p_complete FROM work_packages SQL end + def create_temporary_depth_table + execute(<<~SQL.squish) + CREATE UNLOGGED TABLE temp_work_package_depth AS + WITH RECURSIVE + work_package_depth AS ( + /* Base case: Leaves (work packages with no children) */ + SELECT + wp.id, + wp.parent_id, + 0 AS depth + FROM + temp_wp_progress_values wp + WHERE + NOT EXISTS ( + SELECT + 1 + FROM + temp_wp_progress_values c + WHERE + c.parent_id = wp.id + ) + UNION ALL + /* Recursive case: Parents */ + SELECT + wp.parent_id AS id, + wp2.parent_id, + wpd.depth + 1 AS depth + FROM + work_packages wp + INNER JOIN work_package_depth wpd ON wp.id = wpd.id + INNER JOIN temp_wp_progress_values wp2 ON wp.parent_id = wp2.id + WHERE + wp.parent_id IS NOT NULL + ) + SELECT + id, + depth + FROM + work_package_depth + SQL + end + def drop_temporary_total_percent_complete_table execute(<<~SQL.squish) - DROP TABLE temp_wp_progress_values + DROP TABLE IF EXISTS temp_wp_progress_values + SQL + end + + def drop_temporary_depth_table + execute(<<~SQL.squish) + DROP TABLE IF EXISTS temp_work_package_depth SQL end @@ -223,6 +286,7 @@ def copy_total_percent_complete_values_to_work_packages ) RETURNING work_packages.id SQL + results.column_values(0) end diff --git a/config/locales/en.yml b/config/locales/en.yml index c8f52f7b5079..f9a9845c5397 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1916,7 +1916,7 @@ en: status_changed: "Status '%{status_name}'" system_update: "OpenProject system update:" total_percent_complete_mode_changed_to_work_weighted_average: "Calculation of % Complete totals now weighted by Work." - + total_percent_complete_mode_changed_to_simple_average: "Calculation of % Complete totals now based on a simple average of only % Complete values." cause_descriptions: work_package_predecessor_changed_times: by changes to predecessor %{link} work_package_parent_changed_times: by changes to parent %{link} @@ -1948,7 +1948,8 @@ en: This is a maintenance task and can be safely ignored. total_percent_complete_mode_changed_to_work_weighted_average: >- Child work packages without Work are ignored. - + total_percent_complete_mode_changed_to_simple_average: >- + Work values of child work packages are ignored. links: configuration_guide: "Configuration guide" get_in_touch: "You have questions? Get in touch with us." diff --git a/lib/open_project/journal_formatter/cause.rb b/lib/open_project/journal_formatter/cause.rb index 1674bf78ef06..6148cbf82f9b 100644 --- a/lib/open_project/journal_formatter/cause.rb +++ b/lib/open_project/journal_formatter/cause.rb @@ -79,6 +79,8 @@ def cause_description progress_mode_changed_to_status_based_message when "total_percent_complete_mode_changed_to_work_weighted_average" total_percent_complete_mode_changed_to_work_weighted_average_message + when "total_percent_complete_mode_changed_to_simple_average" + total_percent_complete_mode_changed_to_simple_average_message else related_work_package_changed_message end @@ -149,6 +151,10 @@ def total_percent_complete_mode_changed_to_work_weighted_average_message I18n.t("journals.cause_descriptions.total_percent_complete_mode_changed_to_work_weighted_average") end + def total_percent_complete_mode_changed_to_simple_average_message + I18n.t("journals.cause_descriptions.total_percent_complete_mode_changed_to_simple_average") + end + def related_work_package_changed_message related_work_package = WorkPackage.includes(:project).visible(User.current).find_by(id: cause["work_package_id"]) diff --git a/spec/controllers/admin/settings/progress_tracking_controller_spec.rb b/spec/controllers/admin/settings/progress_tracking_controller_spec.rb index b65bcae1f276..0b6f89bbbee3 100644 --- a/spec/controllers/admin/settings/progress_tracking_controller_spec.rb +++ b/spec/controllers/admin/settings/progress_tracking_controller_spec.rb @@ -111,8 +111,7 @@ } } expect(WorkPackages::Progress::ApplyTotalPercentCompleteModeChangeJob) - .to have_been_enqueued.with(old_mode: "simple_average", - new_mode: "work_weighted_average", + .to have_been_enqueued.with(mode: "work_weighted_average", cause_type: "total_percent_complete_mode_changed_to_work_weighted_average") perform_enqueued_jobs @@ -120,4 +119,26 @@ expect(Setting.total_percent_complete_mode).to eq("work_weighted_average") end end + + context "when changing total percent complete mode from work-weighted average to simple average" do + before do + Setting.total_percent_complete_mode = "work_weighted_average" + end + + it "starts a job to update work packages' total % complete values" do + patch "update", + params: { + settings: { + total_percent_complete_mode: "simple_average" + } + } + expect(WorkPackages::Progress::ApplyTotalPercentCompleteModeChangeJob) + .to have_been_enqueued.with(mode: "simple_average", + cause_type: "total_percent_complete_mode_changed_to_simple_average") + + perform_enqueued_jobs + + expect(Setting.total_percent_complete_mode).to eq("simple_average") + end + end end 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 03caf455e814..4ad6cae47099 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 @@ -72,7 +72,7 @@ def expect_performing_job_changes(from:, to:, context "when changing from simple average to work weighted average mode", with_settings: { total_percent_complete_mode: "work_weighted_average" } do context "on a single-level hierarchy" do - it "updates the total % complete of the work packages" do + it "does not update the total % complete of the work packages" do expect_performing_job_changes( mode: "work_weighted_average", from: <<~TABLE, @@ -139,10 +139,10 @@ def expect_performing_job_changes(from:, to:, mode: "work_weighted_average", from: <<~TABLE, hierarchy | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete - parent | | | | | 40% | 63% + parent | | | | | 40% | 65% child1 | | | | | 100% | child2 | | | | | 40% | - child3 | | | | | 100% | 70% + child3 | | | | | 100% | 80% grandchild1 | | | | | 40% | grandchild2 | | | | | 100% | TABLE @@ -225,6 +225,163 @@ def expect_performing_job_changes(from:, to:, end end + context "when changing from work weighted average to simple average mode", + with_settings: { total_percent_complete_mode: "simple_average" } do + context "on a single-level hierarchy" do + it "does not update the total % complete of the work packages" do + expect_performing_job_changes( + mode: "simple_average", + from: <<~TABLE, + hierarchy | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + flat_wp_1 | 10h | | 6h | | 40% | + flat_wp_2 | 5h | | 3h | | 60% | + TABLE + to: <<~TABLE + subject | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + flat_wp_1 | 10h | | 6h | | 40% | + flat_wp_2 | 5h | | 3h | | 60% | + TABLE + ) + end + end + + context "on a two-level hierarchy with parents having total values" do + it "updates the total % complete of parent work packages" do + expect_performing_job_changes( + mode: "simple_average", + from: <<~TABLE, + hierarchy | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | 10h | 30h | 6h | 6h | 40% | 80% + child1 | 15h | | 0h | | 100% | + child2 | | | | | 40% | + child3 | 5h | | 0h | | 100% | + TABLE + to: <<~TABLE + subject | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | 10h | 30h | 6h | 6h | 40% | 70% + child1 | 15h | | 0h | | 100% | + child2 | | | | | 40% | + child3 | 5h | | 0h | | 100% | + TABLE + ) + end + end + + context "on a two-level hierarchy with only % complete values set" do + it "sets the % complete value from parents" do + expect_performing_job_changes( + mode: "simple_average", + from: <<~TABLE, + hierarchy | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | | | | | 40% | + child1 | | | | | 100% | + child2 | | | | | 40% | + child3 | | | | | 100% | + TABLE + to: <<~TABLE + subject | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | | | | | 40% | 70% + child1 | | | | | 100% | + child2 | | | | | 40% | + child3 | | | | | 100% | + TABLE + ) + end + end + + context "on a multi-level hierarchy with only % complete values set" do + it "unsets the % complete value from parents" do + expect_performing_job_changes( + mode: "simple_average", + from: <<~TABLE, + hierarchy | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | | | | | 40% | + child1 | | | | | 100% | + child2 | | | | | 40% | + child3 | | | | | 100% | + grandchild1 | | | | | 40% | + grandchild2 | | | | | 100% | + TABLE + to: <<~TABLE + subject | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | | | | | 40% | 65% + child1 | | | | | 100% | + child2 | | | | | 40% | + child3 | | | | | 100% | 80% + grandchild1 | | | | | 40% | + grandchild2 | | | | | 100% | + TABLE + ) + end + end + + context "on a multi-level hierarchy with work and remaining work values set" do + it "updates the total % complete of parent work packages irrelevant of work values" do + expect_performing_job_changes( + mode: "simple_average", + from: <<~TABLE, + hierarchy | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | 10h | 50h | 6h | 6h | 40% | 88% + child1 | 15h | | 0h | | 100% | + child2 | | | | | 40% | + child3 | 5h | 25h | 0h | 0h | 100% | 100% + grandchild1 | | | | | 40% | + grandchild2 | 20h | | 0h | | 100% | + TABLE + to: <<~TABLE + subject | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | 10h | 50h | 6h | 6h | 40% | 65% + child1 | 15h | | 0h | | 100% | + child2 | | | | | 40% | + child3 | 5h | 25h | 0h | 0h | 100% | 80% + grandchild1 | | | | | 40% | + grandchild2 | 20h | | 0h | | 100% | + TABLE + ) + end + end + + describe "journal entries" do + # rubocop:disable RSpec/ExampleLength + it "creates journal entries for the modified work packages" do + parent, child1, child2, child3, grandchild1, grandchild2 = expect_performing_job_changes( + cause_type: "total_percent_complete_mode_changed_to_simple_average", + mode: "simple_average", + from: <<~TABLE, + hierarchy | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | 10h | 50h | 6h | 6h | 40% | 88% + child1 | 15h | | 0h | | 100% | + child2 | | | | | 40% | + child3 | 5h | 25h | 0h | 0h | 100% | 100% + grandchild1 | | | | | 40% | + grandchild2 | 20h | | 0h | | 100% | + TABLE + to: <<~TABLE + subject | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | 10h | 50h | 6h | 6h | 40% | 65% + child1 | 15h | | 0h | | 100% | + child2 | | | | | 40% | + child3 | 5h | 25h | 0h | 0h | 100% | 80% + grandchild1 | | | | | 40% | + grandchild2 | 20h | | 0h | | 100% | + TABLE + ) + [parent, child3].each do |work_package| + expect(work_package.journals.count).to eq 2 + last_journal = work_package.last_journal + expect(last_journal.user).to eq(User.system) + expect(last_journal.cause_type).to eq("total_percent_complete_mode_changed_to_simple_average") + end + + # unchanged => no new journals + [child1, child2, grandchild1, grandchild2].each do |work_package| + expect(work_package.journals.count).to eq 1 + end + end + # rubocop:enable RSpec/ExampleLength + end + end + context "with errors during job execution" do let_work_packages(<<~TABLE) subject | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete