references/openproject
1
0
Fork 0
mirror of https://github.com/opf/openproject.git synced 2026-06-14 03:30:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

Migrate progress values for disabled and work-based modes

Browse Source 
This is not complete yet:
- The values are migrated for work-based and disabled mode.
- The disabled mode is not allowed anymore (in the setting definition
  allowed values).
- The values still need to be migrated for status-based mode.
- The totals still need to be recomputed.
- The administration UI still needs to have the disabled mode removed,
  and text changed.
- The journals still need to be skipped if nothing has changed.
- The journal `cause` text still needs to be more precise.
This commit is contained in:
Christophe Bliard
2024-04-03 18:09:13 +02:00
parent ec6628514f
commit acbace64e7
8 changed files with 651 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/workers/work_packages/update_progress_job.rb
+164
View File
@@ -0,0 +1,164 @@
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) 2012-2024 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 WorkPackages::UpdateProgressJob < ApplicationJob
queue_with_priority :default
def perform(previous_mode: nil)
if previous_mode == "disabled"
unset_all_percent_complete_values
end
fix_remaining_work_set_with_100p_complete
fix_remaining_work_exceeding_work
fix_only_work_being_set
fix_only_remaining_work_being_set
derive_unset_remaining_work_from_work_and_p_complete
derive_unset_work_from_remaining_work_and_p_complete
derive_p_complete_from_work_and_remaining_work
create_journals_for_updated_work_packages
end
private
def unset_all_percent_complete_values
execute_sql_update(<<~SQL.squish)
UPDATE work_packages
SET done_ratio = NULL
WHERE done_ratio IS NOT NULL
SQL
end
def fix_remaining_work_set_with_100p_complete
execute_sql_update(<<~SQL.squish)
UPDATE work_packages
SET estimated_hours = remaining_hours,
remaining_hours = 0
WHERE estimated_hours IS NULL
AND remaining_hours > 0
AND done_ratio = 100
SQL
end
def fix_remaining_work_exceeding_work
# avoid a division by zero when work and remaining work are both zero in
# `#derive_p_complete_from_work_and_remaining_work` method.
execute_sql_update(<<~SQL.squish)
UPDATE work_packages
SET done_ratio = 0
WHERE remaining_hours = estimated_hours
SQL
execute_sql_update(<<~SQL.squish)
UPDATE work_packages
SET remaining_hours = estimated_hours,
done_ratio = 0
WHERE remaining_hours > estimated_hours
AND done_ratio IS NULL
SQL
execute_sql_update(<<~SQL.squish)
UPDATE work_packages
SET remaining_hours = ROUND((estimated_hours - (estimated_hours * done_ratio / 100.0))::numeric, 2)
WHERE remaining_hours > estimated_hours
AND done_ratio IS NOT NULL
SQL
end
def fix_only_work_being_set
execute_sql_update(<<~SQL.squish)
UPDATE work_packages
SET remaining_hours = estimated_hours,
done_ratio = 0
WHERE estimated_hours IS NOT NULL
AND remaining_hours IS NULL
AND done_ratio IS NULL
SQL
end
def fix_only_remaining_work_being_set
execute_sql_update(<<~SQL.squish)
UPDATE work_packages
SET estimated_hours = remaining_hours,
done_ratio = 0
WHERE estimated_hours IS NULL
AND remaining_hours IS NOT NULL
AND done_ratio IS NULL
SQL
end
def derive_unset_remaining_work_from_work_and_p_complete
execute_sql_update(<<~SQL.squish)
UPDATE work_packages
SET remaining_hours = ROUND((estimated_hours - (estimated_hours * done_ratio / 100.0))::numeric, 2)
WHERE estimated_hours IS NOT NULL
AND remaining_hours IS NULL
AND done_ratio IS NOT NULL
SQL
end
def derive_unset_work_from_remaining_work_and_p_complete
execute_sql_update(<<~SQL.squish)
UPDATE work_packages
SET estimated_hours = ROUND((remaining_hours * 100 / (100 - done_ratio))::numeric, 2)
WHERE estimated_hours IS NULL
AND remaining_hours IS NOT NULL
AND done_ratio IS NOT NULL
SQL
end
def derive_p_complete_from_work_and_remaining_work
execute_sql_update(<<~SQL.squish)
UPDATE work_packages
SET done_ratio = (estimated_hours - remaining_hours) * 100 / estimated_hours
WHERE estimated_hours IS NOT NULL
AND remaining_hours IS NOT NULL
AND (
done_ratio IS NULL OR (estimated_hours > remaining_hours)
)
SQL
end
def create_journals_for_updated_work_packages
WorkPackage.where(id: updated_work_package_ids).find_each do |work_package|
Journals::CreateService.new(work_package, system_user)
.call(cause: { type: "system_update", feature: "progress_calculation_changed" })
end
end
def execute_sql_update(sql)
result = ActiveRecord::Base.connection.execute("#{sql} RETURNING id")
updated_work_package_ids.merge(result.field_values("id"))
end
def updated_work_package_ids
@updated_work_package_ids ||= Set.new
end
def system_user
@system_user ||= User.system
end
end
config/constants/settings/definition.rb
+1 -1
View File
@@ -1107,7 +1107,7 @@ module Settings
},
work_package_done_ratio: {
default: "field",
allowed: %w[field status disabled]
allowed: %w[field status]
},
work_packages_projects_export_limit: {
default: 500
config/locales/en.yml
+2
View File
@@ -1687,6 +1687,8 @@ Project attributes and sections are defined in the <a href=%{admin_settings_url}
file_links_journal: >
From now on, activity related to file links (files stored in external storages) will appear here in the
Activity tab. The following represent activity concerning links that already existed:
progress_calculation_changed: >
Progress calculation updated.
links:
configuration_guide: "Configuration guide"
db/migrate/20240402072213_update_progress_calculation.rb
+23
View File
@@ -0,0 +1,23 @@
class UpdateProgressCalculation < ActiveRecord::Migration[7.1]
# See https://community.openproject.org/wp/40749 for migration details
def up
if progress_calculation_mode == "disabled"
set_progress_calculation_mode_to_work_based
previous_mode = "disabled"
end
perform_method = Rails.env.production? ? :perform_later : :perform_now
WorkPackages::UpdateProgressJob.public_send(perform_method, previous_mode:)
end
def progress_calculation_mode
ActiveRecord::Base.connection
.execute("SELECT value FROM settings WHERE name = 'work_package_done_ratio'")
.first
&.fetch("value", nil)
end
def set_progress_calculation_mode_to_work_based
ActiveRecord::Base.connection.execute("UPDATE settings SET value = 'field' WHERE name = 'work_package_done_ratio'")
end
end
spec/factories/work_package_factory.rb
+8 -2
View File
@@ -127,11 +127,17 @@ FactoryBot.define do
end
set_done_ratios = ->(work_package, _evaluator) do
if work_package.estimated_hours.present? && work_package.remaining_hours.present?
if work_package.estimated_hours.present? &&
work_package.remaining_hours.present? &&
work_package.done_ratio.nil? &&
work_package.estimated_hours >= work_package.remaining_hours
work_package.done_ratio = (work_package.estimated_hours - work_package.remaining_hours) \
/ work_package.estimated_hours.to_f * 100
end
if work_package.derived_estimated_hours.present? && work_package.derived_remaining_hours.present?
if work_package.derived_estimated_hours.present? &&
work_package.derived_remaining_hours.present? &&
work_package.derived_done_ratio.nil? &&
work_package.derived_estimated_hours >= work_package.derived_remaining_hours
work_package.derived_done_ratio = (work_package.derived_estimated_hours - work_package.derived_remaining_hours) \
/ work_package.derived_estimated_hours.to_f * 100
end
spec/migrations/update_progress_calculation_spec.rb
+448
View File
@@ -0,0 +1,448 @@
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) 2012-2024 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.
#++
require "spec_helper"
require Rails.root.join("db/migrate/20240402072213_update_progress_calculation.rb")
RSpec.describe UpdateProgressCalculation, type: :model do
# Silencing migration logs, since we are not interested in that during testing
subject(:run_migration) { ActiveRecord::Migration.suppress_messages { described_class.new.up } }
shared_let(:author) { create(:user) }
shared_let(:priority) { create(:priority, name: "Normal") }
shared_let(:project) { create(:project, name: "Main project") }
shared_let(:status_new) { create(:status, name: "New") }
before_all do
set_factory_default(:user, author)
set_factory_default(:priority, priority)
set_factory_default(:project, project)
set_factory_default(:project_with_types, project)
set_factory_default(:status, status_new)
end
# let_work_packages(<<~TABLE)
# hierarchy | work | remaining work | % complete
# wp all unset | | |
# wp only w set | 10h | |
# wp only rw set | | 4h |
# wp only pc set | | | 60%
# wp both w and rw set | 10h | 4h |
# wp both w and pc set | 10h | | 60%
# wp both rw and pc set | | 4h | 60%
# wp both w and big rw set | 10h | 99h |
# wp all set consistent | 10h | 4h | 60%
# wp all set inconsistent | 10h | 1h | 10%
# wp all set big wp | 10h | 99h | 60%
# wp all set big wp round | 8h | 99h | 70%
# TABLE
def expect_migrates(from:, to:)
table = create_table(from)
run_migration
table.work_packages.map(&:reload)
expect_work_packages(table.work_packages, to)
table.work_packages
end
context "when in disabled mode" do
before do
Setting.work_package_done_ratio = "disabled"
end
it "changes progress calculation mode to work-based" do
run_migration
expect(Setting.find_by(name: "work_package_done_ratio")).to have_attributes(value: "field")
end
it "unset all % complete values and creates a journal entry for it" do
work_packages = expect_migrates(
from: <<~TABLE,
hierarchy | work | remaining work | % complete
wp only pc set | | | 60%
TABLE
to: <<~TABLE
hierarchy | work | remaining work | % complete
wp only pc set | | |
TABLE
)
wp_only_pc_set = work_packages.first
expect(wp_only_pc_set.last_journal.details).to include("done_ratio" => [60, nil])
expect(wp_only_pc_set.last_journal.cause).to eq("type" => "system_update",
"feature" => "progress_calculation_changed")
end
context "when all unset" do
it "does nothing, everything is still unset and no journal is created" do
work_packages = expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp all unset | | |
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp all unset | | |
TABLE
)
wp_all_unset = work_packages.first
# no new journals as nothing changed
expect(wp_all_unset.journals.count).to eq(1)
end
end
context "when Work is set and Remaining work is unset" do
it "sets Remaining work to Work, and % Complete to 0%" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp only w set | 10h | |
wp both w and pc set | 10h | | 60%
wp only w set 0h | 0h | |
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp only w set | 10h | 10h | 0%
wp both w and pc set | 10h | 10h | 0%
wp only w set 0h | 0h | 0h | 0%
TABLE
)
end
end
context "when Work is unset and Remaining work is set" do
it "sets Work to Remaining work, and % Complete to 0%" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp only rw set | | 4h |
wp both rw and pc set | | 4h | 60%
wp only rw set 0h | | 0h |
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp only rw set | 4h | 4h | 0%
wp both rw and pc set | 4h | 4h | 0%
wp only rw set 0h | 0h | 0h | 0%
TABLE
)
end
end
context "when Work is greater than Remaining work" do
it "derives % Complete value" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp both w and rw set | 10h | 4h |
wp both w and rw set 0h | 10h | 0h |
wp all set consistent | 10h | 4h | 60%
wp all set inconsistent | 10h | 1h | 10%
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp both w and rw set | 10h | 4h | 60%
wp both w and rw set 0h | 10h | 0h | 100%
wp all set consistent | 10h | 4h | 60%
wp all set inconsistent | 10h | 1h | 90%
TABLE
)
end
end
context "when Work is equal to Remaining work" do
it "sets % Complete to 0%" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp both w and rw set | 10h | 10h |
wp both w and rw set 0h | 0h | 0h |
wp all set inconsistent | 10h | 10h | 60%
wp all set inconsistent 0h| 0h | 0h | 60%
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp both w and rw set | 10h | 10h | 0%
wp both w and rw set 0h | 0h | 0h | 0%
wp all set inconsistent | 10h | 10h | 0%
wp all set inconsistent 0h| 0h | 0h | 0%
TABLE
)
end
end
context "when Remaining work is greater than Work" do
it "sets Remaining work to Work, and % Complete to 0%" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp both w and big rw set | 10h | 99h |
wp all set big wp | 10h | 99h | 60%
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp both w and big rw set | 10h | 10h | 0%
wp all set big wp | 10h | 10h | 0%
TABLE
)
end
end
end
###
context "when in work-based mode" do
before do
Setting.work_package_done_ratio = "field"
end
context "when all unset" do
it "does nothing and everything is still unset" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp all unset | | |
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp all unset | | |
TABLE
)
end
end
context "when only Work is set" do
it "sets Remaining work to Work, and % Complete to 0%" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp only w set | 10h | |
wp only w set 0h | 0h | |
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp only w set | 10h | 10h | 0%
wp only w set 0h | 0h | 0h | 0%
TABLE
)
end
end
context "when only Remaining work is set" do
it "sets Work to Remaining work, and % Complete to 0%" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp only rw set | | 4h |
wp only rw set 0h | | 0h |
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp only rw set | 4h | 4h | 0%
wp only rw set 0h | 0h | 0h | 0%
TABLE
)
end
end
context "when only % Complete is set" do
it "does nothing and % Complete is kept" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp only pc set | | | 60%
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp only pc set | | | 60%
TABLE
)
end
end
context "when Work and % Complete are set, and Remaining work is unset" do
it "derives Remaining work from Work and % Complete" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp both w and pc set | 10h | | 60%
wp both w and pc set 0h | 0h | | 0%
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp both w and pc set | 10h | 4h | 60%
wp both w and pc set 0h | 0h | 0h | 0%
TABLE
)
end
end
context "when Remaining work and % Complete are set, and Work is unset" do
it "derives Work from Remaining work and % Complete" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp both rw and pc set | | 4h | 60%
wp both rw and pc set 0% | | 4h | 0%
wp both rw and pc set dec | | 4h | 67%
wp both rw and pc set 0h | | 0h | 0%
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp both rw and pc set | 10h | 4h | 60%
wp both rw and pc set 0% | 4h | 4h | 0%
wp both rw and pc set dec | 12.12h | 4h | 67%
wp both rw and pc set 0h | 0h | 0h | 0%
TABLE
)
end
end
context "when Remaining work is set, % Complete is 100%, and Work is unset" do
it "sets Work to Remaining work, sets Remaining work to 0h and keep % Complete" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp both rw and pc set 100% | | 4h | 100%
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp both rw and pc set 100% | 4h | 0h | 100%
TABLE
)
end
end
context "when Work is greater than Remaining work and % Complete is unset" do
it "derives % Complete value" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp both w and rw set | 10h | 4h |
wp both w and rw set 0h | 10h | 0h |
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp both w and rw set | 10h | 4h | 60%
wp both w and rw set 0h | 10h | 0h | 100%
TABLE
)
end
end
context "when Work is equal to Remaining work and % Complete is unset" do
it "sets % Complete to 0%" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp both w and rw set | 10h | 10h |
wp both w and rw set 0h | 0h | 0h |
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp both w and rw set | 10h | 10h | 0%
wp both w and rw set 0h | 0h | 0h | 0%
TABLE
)
end
end
context "when Remaining work is greater than Work and % Complete is unset" do
it "sets Remaining work to Work, and % Complete to 0%" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp both w and big rw set | 10h | 99h |
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp both w and big rw set | 10h | 10h | 0%
TABLE
)
end
end
context "when Work is greater than Remaining work and % Complete is set" do
it "derives % Complete value" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp all set consistent | 10h | 4h | 60%
wp all set inconsistent | 10h | 1h | 10%
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp all set consistent | 10h | 4h | 60%
wp all set inconsistent | 10h | 1h | 90%
TABLE
)
end
end
context "when Work is equal to Remaining work and % Complete is set" do
it "sets % Complete to 0%" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp all set consistent | 10h | 10h | 0%
wp all set inconsistent | 10h | 10h | 60%
wp all set 0h | 0h | 0h | 55%
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp all set consistent | 10h | 10h | 0%
wp all set inconsistent | 10h | 10h | 0%
wp all set 0h | 0h | 0h | 0%
TABLE
)
end
end
context "when Remaining work is greater than Work and % Complete is set" do
it "computes Remaining work from Work and % Complete" do
expect_migrates(
from: <<~TABLE,
subject | work | remaining work | % complete
wp all set big wp | 10h | 99h | 60%
wp all set big wp round | 8h | 2.4h | 70%
TABLE
to: <<~TABLE
subject | work | remaining work | % complete
wp all set big wp | 10h | 4h | 60%
# would be 2.4000000000000004h without rounding
wp all set big wp round | 8h | 2.4h | 70%
TABLE
)
end
end
end
end
spec/support/table_helpers/table.rb
+4
View File
@@ -37,6 +37,10 @@ module TableHelpers
@work_packages_by_identifier[name]
end
def work_packages
@work_packages_by_identifier.values
end
private
def normalize_name(name)
spec/support/table_helpers/table_parser.rb
+1 -1
View File
@@ -32,7 +32,7 @@ module TableHelpers
class TableParser
def parse(representation)
headers, *rows = representation.split("\n")
headers = headers.split("|")
headers = (headers || "").split("|")
rows = rows.filter_map { |row| parse_row(row, headers) }
work_packages_data = rows.map.with_index do |row, index|
{