Automatically correct progress values when possible

https://community.openproject.org/wp/66592

In case the values ever get corrupted, instead of annoying the user with
a validation message which prevents saving the work package, it tries to
automatically correct the values.

The autocorrection is possible only if all values are set and are valid.

app/contracts/work_packages/base_contract.rb

@@ -398,9 +398,7 @@ module WorkPackages
     end
 
     def validate_percent_complete_matches_work_and_remaining_work
-      return if percent_complete_derivation_unapplicable?
-
-      if !consistent_progress_values?(work:, remaining_work:, percent_complete:)
+      if correctable_percent_complete_value?(work:, remaining_work:, percent_complete:)
         errors.add(:done_ratio, :does_not_match_work_and_remaining_work)
       end
     end
@@ -476,12 +474,6 @@ module WorkPackages
       percent_complete.nil?
     end
 
-    def percent_complete_derivation_unapplicable?
-      WorkPackage.status_based_mode? || # only applicable in work-based mode
-        work_empty? || remaining_work_empty? || percent_complete_empty? || # only applicable if all 3 values are set
-        work == 0 || percent_complete == 100 # only applicable if not in special cases leading to divisions by zero
-    end
-
     def validate_no_reopen_on_closed_version
       if model.version_id && model.reopened? && model.version.closed?
         errors.add :base, I18n.t(:error_can_not_reopen_work_package_on_closed_version)

app/services/work_packages/set_attributes_service/derive_progress_values_work_based.rb

@@ -71,12 +71,15 @@ class WorkPackages::SetAttributesService
     end
 
     def derive_remaining_work?
-      remaining_work_not_provided_by_user? && (work_changed? || percent_complete_changed?)
+      (remaining_work_not_provided_by_user? && (work_changed? || percent_complete_changed?)) \
+        || fixable_remaining_work?
     end
 
     def derive_percent_complete?
-      percent_complete_not_provided_by_user? && (work_changed? || remaining_work_changed?) \
-        && !skip_percent_complete_derivation
+      return false if skip_percent_complete_derivation
+
+      (percent_complete_not_provided_by_user? && (work_changed? || remaining_work_changed?)) \
+        || fixable_percent_complete?
     end
 
     def set_complete
@@ -170,5 +173,21 @@ class WorkPackages::SetAttributesService
         || (remaining_work_came_from_user? && percent_complete_came_from_user?) \
         || (remaining_work_empty? && remaining_work_came_from_user?)
     end
+
+    def fixable_remaining_work?
+      no_progress_value_provided_by_user? \
+        && correctable_remaining_work_value?(work:, remaining_work:, percent_complete:)
+    end
+
+    def fixable_percent_complete?
+      no_progress_value_provided_by_user? \
+        && correctable_percent_complete_value?(work:, remaining_work:, percent_complete:)
+    end
+
+    def no_progress_value_provided_by_user?
+      work_not_provided_by_user? \
+        && remaining_work_not_provided_by_user? \
+        && percent_complete_not_provided_by_user?
+    end
   end
 end

app/services/work_packages/set_attributes_service/progress_values_calculations.rb

@@ -64,10 +64,44 @@ class WorkPackages::SetAttributesService
       end
     end
 
-    def consistent_progress_values?(work:, remaining_work:, percent_complete:)
+    # Check if the remaining work value is wrong and can be corrected regarding
+    # work and % complete values.
+    #
+    # In most cases, it's not remaining work but % complete which must change.
+    # The only case where remaining work must be corrected is when work is set,
+    # % complete is 100% and remaining work is not 0h. In this case this method
+    # returns `true`.
+    def correctable_remaining_work_value?(work:, remaining_work:, percent_complete:)
+      work.present? && remaining_work != 0 && percent_complete == 100
+    end
+
+    # Check if the % complete value is wrong and can be corrected regarding work
+    # and remaining work values.
+    #
+    # Returns `false` if the percent complete is the same as the one calculated
+    # from work and remaining work, or if the remaining work is the same as the
+    # one calculated from work and percent complete.
+    #
+    # Returns `false` also if the percent complete value cannot be calculated
+    # because other values are missing or out of bounds.
+    #
+    # Returns `false` also in the special case where % complete is 100% and
+    # remaining work greater than 0h. In this case, it's remaining work which needs to
+    # be corrected to 0h.
+    def correctable_percent_complete_value?(work:, remaining_work:, percent_complete:)
+      return false unless percent_complete_calculation_applicable?(work:, remaining_work:, percent_complete:)
+
       # Check if one of provided remaining_work or percent_complete matches the calculated one
-      percent_complete == calculate_percent_complete(work:, remaining_work:) \
-        || remaining_work == calculate_remaining_work(work:, percent_complete:)
+      percent_complete != calculate_percent_complete(work:, remaining_work:) \
+        && remaining_work != calculate_remaining_work(work:, percent_complete:)
+    end
+
+    def percent_complete_calculation_applicable?(work:, remaining_work:, percent_complete:)
+      WorkPackage.work_based_mode? && # only applicable in work-based mode
+        work && remaining_work && percent_complete && # only applicable if all 3 values are set
+        work != 0 && # only applicable if not leading to divisions by zero
+        percent_complete != 100 && # keep 100% complete as is
+        remaining_work >= 0 && work >= remaining_work # only applicable if positive and work is greater than remaining work
     end
   end
 end

spec/services/work_packages/set_attributes_service/be_consistent_progress_values_matcher.rb

@@ -1,90 +0,0 @@
-# frozen_string_literal: true
-
-#-- 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 OpenProject GmbH.
-#
-# See COPYRIGHT and LICENSE files for more details.
-#++
-
-# Custom matcher to check if progress values are consistent using
-# WorkPackages::SetAttributesService::ProgressValuesCalculations#consistent_progress_values?
-#
-# The helper helps at telling the acceptable range of values for the percent
-# complete and/or remaining work when it fails.
-#
-# Usage:
-#   expect(work: 10, remaining_work: 0, percent_complete: 100).to be_consistent_progress_values
-#   expect(work: 10, remaining_work: 5, percent_complete: 50).to be_consistent_progress_values
-RSpec::Matchers.define :be_consistent_progress_values do
-  match do |progress_values|
-    work, remaining_work, percent_complete = progress_values.values_at(:work, :remaining_work, :percent_complete)
-
-    # Create a dummy class to access the ProgressValuesCalculations module
-    dummy_class = Class.new { extend WorkPackages::SetAttributesService::ProgressValuesCalculations }
-
-    dummy_class.consistent_progress_values?(work:, remaining_work:, percent_complete:)
-  end
-
-  failure_message do |progress_values|
-    work, remaining_work, percent_complete = progress_values.values_at(:work, :remaining_work, :percent_complete)
-
-    dummy_class = Class.new { extend WorkPackages::SetAttributesService::ProgressValuesCalculations }
-    expected_percent_complete = dummy_class.derive_percent_complete(work:, remaining_work:)
-    expected_remaining_work = dummy_class.derive_remaining_work(work:, percent_complete:)
-
-    <<~MESSAGE
-      expected progress values to be consistent:
-        work: #{work}h
-        remaining_work: #{remaining_work}h
-        percent_complete: #{percent_complete}%
-
-      but they are not consistent
-        either derived percent_complete should be #{expected_percent_complete}% with work=#{work}h and remaining_work=#{remaining_work}h
-        or derived remaining_work should be #{expected_remaining_work}h with work=#{work}h and percent_complete=#{percent_complete}%
-    MESSAGE
-  end
-
-  failure_message_when_negated do |progress_values|
-    work, remaining_work, percent_complete = progress_values.values_at(:work, :remaining_work, :percent_complete)
-
-    dummy_class = Class.new { extend WorkPackages::SetAttributesService::ProgressValuesCalculations }
-    expected_percent_complete = dummy_class.derive_percent_complete(work:, remaining_work:)
-    expected_remaining_work = dummy_class.derive_remaining_work(work:, percent_complete:)
-
-    <<~MESSAGE
-      expected progress values to NOT be consistent:
-        work: #{work}h
-        remaining_work: #{remaining_work}h
-        percent_complete: #{percent_complete}%
-
-      but they are actually consistent:
-        derived percent_complete is #{expected_percent_complete}% with work=#{work}h and remaining_work=#{remaining_work}h
-        derived remaining_work is #{expected_remaining_work}h with work=#{work}h and percent_complete=#{percent_complete}%
-    MESSAGE
-  end
-
-  description do
-    "have consistent progress values"
-  end
-end

spec/services/work_packages/set_attributes_service/derive_progress_values_work_based_spec.rb

@@ -441,6 +441,142 @@ RSpec.describe WorkPackages::SetAttributesService::DeriveProgressValuesWorkBased
     end
   end
 
+  context "given a work package with work, remaining work, and % complete being set " \
+          "with incorrect but fixable % complete value" do
+    before do
+      # remaining hours and % complete are inconsistent with each other
+      work_package.estimated_hours = 100.0
+      work_package.remaining_hours = 30.0 # should be 58h if % complete is 42%
+      work_package.done_ratio = 42 # should be 70% if remaining work is 30h
+      work_package.clear_changes_information
+    end
+
+    context "when nothing is changed by the user" do
+      let(:set_attributes) { {} }
+      let(:expected_derived_attributes) { { done_ratio: 70 } }
+      let(:expected_kept_attributes) { %w[estimated_hours remaining_hours] }
+
+      include_examples "update progress values", description: "derives % complete from work and remaining work",
+                                                 expected_hints: {
+                                                   done_ratio: :derived
+                                                 }
+    end
+
+    context "when something else is changed (like the subject)" do
+      let(:set_attributes) { { subject: "modified subject" } }
+      let(:expected_derived_attributes) { { done_ratio: 70 } }
+      let(:expected_kept_attributes) { %w[estimated_hours remaining_hours] }
+
+      include_examples "update progress values", description: "derives % complete from work and remaining work",
+                                                 expected_hints: {
+                                                   done_ratio: :derived
+                                                 }
+    end
+
+    context "when remaining work is updated by the user" do
+      let(:set_attributes) { { remaining_hours: 25.0 } }
+      let(:expected_derived_attributes) { { done_ratio: 75 } }
+      let(:expected_kept_attributes) { %w[estimated_hours] }
+
+      include_examples "update progress values", description: "derives % complete from work and remaining work",
+                                                 expected_hints: {
+                                                   done_ratio: :derived
+                                                 }
+    end
+
+    context "when work is cleared by the user" do
+      let(:set_attributes) { { estimated_hours: nil } }
+      let(:expected_derived_attributes) { { remaining_hours: nil } }
+      let(:expected_kept_attributes) { %w[done_ratio] }
+
+      include_examples "update progress values", description: "keeps % complete and clears remaining work",
+                                                 expected_hints: {
+                                                   remaining_work: :cleared_because_work_is_empty
+                                                 }
+    end
+
+    context "when % complete is set by the user" do
+      let(:set_attributes) { { done_ratio: 75 } }
+      let(:expected_derived_attributes) { { remaining_hours: 25.0 } }
+      let(:expected_kept_attributes) { %w[estimated_hours] }
+
+      include_examples "update progress values", description: "derives remaining work from work and % complete",
+                                                 expected_hints: {
+                                                   remaining_work: :derived
+                                                 }
+    end
+
+    context "when work is initially 0h and nothing is changed by the user" do
+      let(:set_attributes) { {} }
+      let(:expected_derived_attributes) { {} }
+      let(:expected_kept_attributes) { %w[estimated_hours remaining_hours done_ratio] }
+
+      before do
+        work_package.estimated_hours = 0
+        work_package.clear_changes_information
+      end
+
+      include_examples "update progress values", description: "does not change anything",
+                                                 expected_hints: {}
+    end
+
+    context "when work is initially unset and nothing is changed by the user" do
+      let(:set_attributes) { {} }
+      let(:expected_derived_attributes) { {} }
+      let(:expected_kept_attributes) { %w[estimated_hours remaining_hours done_ratio] }
+
+      before do
+        work_package.estimated_hours = nil
+        work_package.clear_changes_information
+      end
+
+      include_examples "update progress values", description: "does not change anything",
+                                                 expected_hints: {}
+    end
+  end
+
+  context "given a work package with work, remaining work, and % complete being set " \
+          "with incorrect but fixable remaining work value (100% complete with work set)" do
+    before do
+      # remaining hours and % complete are inconsistent with each other
+      work_package.estimated_hours = 10.0
+      work_package.remaining_hours = 5.0 # should be 0h if % complete is 100%
+      work_package.done_ratio = 100
+      work_package.clear_changes_information
+    end
+
+    context "when nothing is changed by the user" do
+      let(:set_attributes) { {} }
+      let(:expected_derived_attributes) { { remaining_hours: 0 } }
+      let(:expected_kept_attributes) { %w[estimated_hours done_ratio] }
+
+      include_examples "update progress values", description: "keeps % complete and sets remaining work to 0h",
+                                                 expected_hints: {
+                                                   remaining_hours: :derived
+                                                 }
+    end
+
+    context "when work is changed by the user" do
+      let(:set_attributes) { { estimated_hours: 12 } }
+      let(:expected_derived_attributes) { { remaining_hours: 7, done_ratio: 42 } }
+
+      include_examples "update progress values", description: "remaining work is increased like work, and % complete is derived",
+                                                 expected_hints: {
+                                                   remaining_hours: { increased_by_delta_like_work: { delta: 2.0 } },
+                                                   done_ratio: :derived
+                                                 }
+    end
+
+    context "when remaining work is changed by the user" do
+      let(:set_attributes) { { remaining_hours: 12 } }
+      let(:expected_derived_attributes) { {} }
+      let(:expected_kept_attributes) { %w[estimated_hours done_ratio] }
+
+      include_examples "update progress values", description: "keeps work and % complete (error to be detected by the contract)",
+                                                 expected_hints: {}
+    end
+  end
+
   context "when % Complete is automatically set to 100% when status is closed",
           with_settings: { percent_complete_on_status_closed: "set_100p" } do
     context "given a work package with a non-closed status" do

spec/services/work_packages/set_attributes_service/have_correctable_progress_value_matchers.rb

@@ -0,0 +1,173 @@
+# frozen_string_literal: true
+
+#-- 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 OpenProject GmbH.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+# Custom matcher to check if remaining work value can be corrected because it is
+# inconsistent with work and % complete as returned by
+# WorkPackages::SetAttributesService::ProgressValuesCalculations#correctable_remaining_work_value?
+#
+# The helper helps at outputting the acceptable range of values for the percent
+# complete and/or remaining work when it fails.
+#
+# Usage:
+#   # 100% complete and 0h remaining work => good values
+#   expect(work: 10, remaining_work: 0, percent_complete: 100)
+#     .not_to have_correctable_remaining_work_value
+#   # 100% complete and not 0h remaining work=> bad remaining work
+#   expect(work: 10, remaining_work: 4, percent_complete: 100)
+#     .to have_correctable_remaining_work_value
+#   # good values
+#   expect(work: 10, remaining_work: 5, percent_complete: 50)
+#     .not_to have_correctable_remaining_work_value
+#   # % complete is the one to be corrected, not remaining work
+#   expect(work: 10, remaining_work: 5, percent_complete: 12)
+#     .not_to have_correctable_remaining_work_value
+RSpec::Matchers.define :have_correctable_remaining_work_value do
+  match do |progress_values|
+    @work, @remaining_work, @percent_complete = progress_values.values_at(:work, :remaining_work, :percent_complete)
+
+    # Create a dummy class to access the ProgressValuesCalculations module
+    dummy_class = Class.new { extend WorkPackages::SetAttributesService::ProgressValuesCalculations }
+
+    dummy_class.correctable_remaining_work_value?(
+      work: @work,
+      remaining_work: @remaining_work,
+      percent_complete: @percent_complete
+    )
+  end
+
+  failure_message do
+    <<~MESSAGE
+      expected remaining work to be correctable:
+        work: #{@work ? "#{@work}h" : '-'}
+        remaining_work: #{@remaining_work ? "#{@remaining_work}h" : '-'}
+        percent_complete: #{@percent_complete ? "#{@percent_complete}%" : '-'}
+    MESSAGE
+  end
+
+  failure_message_when_negated do
+    <<~MESSAGE
+      expected remaining work to not be correctable:
+        work: #{@work ? "#{@work}h" : '-'}
+        remaining_work: #{@remaining_work ? "#{@remaining_work}h" : '-'}
+        percent_complete: #{@percent_complete ? "#{@percent_complete}%" : '-'}
+    MESSAGE
+  end
+
+  description do
+    "have correctable remaining work value"
+  end
+end
+
+# Custom matcher to check if % complete value can be corrected because it is
+# inconsistent with work and remaining work as returned by
+# WorkPackages::SetAttributesService::ProgressValuesCalculations#correctable_percent_complete_value?
+#
+# The helper helps at outputting the acceptable range of values for the percent
+# complete and/or remaining work when it fails.
+#
+# Usage:
+#   expect(work: 10, remaining_work: 0, percent_complete: 0).to have_correctable_percent_complete_value
+#   expect(work: 10, remaining_work: 5, percent_complete: 42).to have_correctable_percent_complete_value
+#   expect(work: 10, remaining_work: 0, percent_complete: 100).not_to have_correctable_percent_complete_value
+#   expect(work: 10, remaining_work: 5, percent_complete: 50).not_to have_correctable_percent_complete_value
+RSpec::Matchers.define :have_correctable_percent_complete_value do
+  match do |progress_values|
+    work, remaining_work, percent_complete = progress_values.values_at(:work, :remaining_work, :percent_complete)
+
+    # Create a dummy class to access the ProgressValuesCalculations module
+    dummy_class = Class.new { extend WorkPackages::SetAttributesService::ProgressValuesCalculations }
+
+    dummy_class.correctable_percent_complete_value?(work:, remaining_work:, percent_complete:)
+  end
+
+  failure_message do |progress_values|
+    work, remaining_work, percent_complete = progress_values.values_at(:work, :remaining_work, :percent_complete)
+
+    dummy_class = Class.new { extend WorkPackages::SetAttributesService::ProgressValuesCalculations }
+    calculation_applicable = dummy_class.percent_complete_calculation_applicable?(work:, remaining_work:, percent_complete:)
+    explanation =
+      if calculation_applicable
+        expected_percent_complete = dummy_class.calculate_percent_complete(work:, remaining_work:)
+        expected_remaining_work = dummy_class.calculate_remaining_work(work:, percent_complete:)
+
+        calculated_percent_complete_explanation =
+          "calculated percent_complete is #{expected_percent_complete}% when work=#{work}h and remaining_work=#{remaining_work}h"
+        calculated_remaining_work_explanation =
+          "calculated remaining_work is #{expected_remaining_work}h when work=#{work}h and percent_complete=#{percent_complete}%"
+
+        if expected_percent_complete == percent_complete
+          correct_derived_explanation = calculated_percent_complete_explanation
+          other_derived_explanation = calculated_remaining_work_explanation
+        else
+          correct_derived_explanation = calculated_remaining_work_explanation
+          other_derived_explanation = calculated_percent_complete_explanation
+        end
+
+        <<~EXPLANATION
+          but it is already correct:
+            #{correct_derived_explanation}
+            (and FYI, #{other_derived_explanation})
+        EXPLANATION
+      else
+        "but % complete cannot be calculated with these progress values"
+      end
+
+    <<~MESSAGE
+      expected percent complete to be correctable:
+        work: #{work ? "#{work}h" : '-'}
+        remaining_work: #{remaining_work ? "#{remaining_work}h" : '-'}
+        percent_complete: #{percent_complete ? "#{percent_complete}%" : '-'}
+
+      #{explanation}
+    MESSAGE
+  end
+
+  failure_message_when_negated do |progress_values|
+    work, remaining_work, percent_complete = progress_values.values_at(:work, :remaining_work, :percent_complete)
+
+    dummy_class = Class.new { extend WorkPackages::SetAttributesService::ProgressValuesCalculations }
+    expected_percent_complete = dummy_class.calculate_percent_complete(work:, remaining_work:)
+    expected_remaining_work = dummy_class.calculate_remaining_work(work:, percent_complete:)
+
+    <<~MESSAGE
+      expected percent complete to not be correctable:
+        work: #{work ? "#{work}h" : '-'}
+        remaining_work: #{remaining_work ? "#{remaining_work}h" : '-'}
+        percent_complete: #{percent_complete ? "#{percent_complete}%" : '-'}
+
+      but % complete can be calculated, and it is not correct:
+        either percent_complete should be #{expected_percent_complete}% when work=#{work}h and remaining_work=#{remaining_work}h
+        or remaining_work should be #{expected_remaining_work}h when work=#{work}h and percent_complete=#{percent_complete}%
+    MESSAGE
+  end
+
+  description do
+    "have correctable percent complete value"
+  end
+end

spec/services/work_packages/set_attributes_service/progress_values_calculations_spec.rb

@@ -29,7 +29,7 @@
 #++
 
 require "rails_helper"
-require_relative "be_consistent_progress_values_matcher"
+require_relative "have_correctable_progress_value_matchers"
 
 RSpec.describe WorkPackages::SetAttributesService::ProgressValuesCalculations do
   let(:dummy_class) { Class.new { extend WorkPackages::SetAttributesService::ProgressValuesCalculations } }
@@ -159,15 +159,62 @@ RSpec.describe WorkPackages::SetAttributesService::ProgressValuesCalculations do
     end
   end
 
-  describe "#consistent_progress_values?" do
-    it "returns true if the progress values are consistent, false otherwise" do
-      expect(work: 10, remaining_work: 0, percent_complete: 100).to be_consistent_progress_values
-      expect(work: 10, remaining_work: 0.5, percent_complete: 95).to be_consistent_progress_values
-      expect(work: 10, remaining_work: 5, percent_complete: 50).to be_consistent_progress_values
-      expect(work: 10, remaining_work: 10, percent_complete: 0).to be_consistent_progress_values
+  describe "#correctable_remaining_work_value?" do
+    it "returns true when work is set, remaining work is not 0h, and % complete is 100%" do
+      expect(work: 10, remaining_work: 11, percent_complete: 100).to have_correctable_remaining_work_value
+      expect(work: 10, remaining_work: 10, percent_complete: 100).to have_correctable_remaining_work_value
+      expect(work: 10, remaining_work: 1, percent_complete: 100).to have_correctable_remaining_work_value
+      expect(work: 10, remaining_work: -1, percent_complete: 100).to have_correctable_remaining_work_value
+      expect(work: 10, remaining_work: nil, percent_complete: 100).to have_correctable_remaining_work_value
 
-      expect(work: 10, remaining_work: 10, percent_complete: 20).not_to be_consistent_progress_values
-      expect(work: 10, remaining_work: 0, percent_complete: 42).not_to be_consistent_progress_values
+      # this one feels weird...
+      expect(work: 0, remaining_work: 10, percent_complete: 100).to have_correctable_remaining_work_value
+    end
+
+    it "returns false when work is not set" do
+      expect(work: nil, remaining_work: 10, percent_complete: 100).not_to have_correctable_remaining_work_value
+    end
+
+    it "returns false when remaining work is 0h" do
+      expect(work: 10, remaining_work: 0, percent_complete: 100).not_to have_correctable_remaining_work_value
+    end
+
+    it "returns false when % complete is not 100%" do
+      expect(work: 10, remaining_work: 10, percent_complete: 42).not_to have_correctable_remaining_work_value
+      expect(work: 10, remaining_work: 10, percent_complete: 0).not_to have_correctable_remaining_work_value
+      expect(work: 10, remaining_work: 10, percent_complete: nil).not_to have_correctable_remaining_work_value
+    end
+  end
+
+  describe "#correctable_percent_complete_value?" do
+    it "returns true when % complete is calculable and does not match the value derived from work and remaining work" do
+      expect(work: 10, remaining_work: 10, percent_complete: 20).to have_correctable_percent_complete_value
+      expect(work: 10, remaining_work: 0, percent_complete: 42).to have_correctable_percent_complete_value
+    end
+
+    it "returns false when % complete is calculable and matches the value derived from work and remaining work" do
+      expect(work: 10, remaining_work: 5, percent_complete: 100).not_to have_correctable_percent_complete_value
+      expect(work: 10, remaining_work: 0.5, percent_complete: 95).not_to have_correctable_percent_complete_value
+      expect(work: 10, remaining_work: 5, percent_complete: 50).not_to have_correctable_percent_complete_value
+      expect(work: 10, remaining_work: 10, percent_complete: 0).not_to have_correctable_percent_complete_value
+    end
+
+    it "returns false when % complete can not be calculated" do
+      # any value  nil => % complete cannot be calculated
+      expect(work: nil, remaining_work: 0, percent_complete: 100).not_to have_correctable_percent_complete_value
+      expect(work: 10, remaining_work: nil, percent_complete: 100).not_to have_correctable_percent_complete_value
+      expect(work: 10, remaining_work: 0, percent_complete: nil).not_to have_correctable_percent_complete_value
+
+      # negative value => % complete cannot be calculated
+      expect(work: -10, remaining_work: 0, percent_complete: 100).not_to have_correctable_percent_complete_value
+      expect(work: 10, remaining_work: -3, percent_complete: 100).not_to have_correctable_percent_complete_value
+
+      # work = 0h => % complete cannot be calculated (division by zero)
+      expect(work: 0, remaining_work: 0, percent_complete: 100).not_to have_correctable_percent_complete_value
+      expect(work: 0, remaining_work: 0, percent_complete: 0).not_to have_correctable_percent_complete_value
+
+      # work < remaining work => % complete cannot be calculated
+      expect(work: 10, remaining_work: 11, percent_complete: 100).not_to have_correctable_percent_complete_value
     end
 
     it "tolerates a range of % complete values when work and remaining work precision " \
@@ -177,16 +224,16 @@ RSpec.describe WorkPackages::SetAttributesService::ProgressValuesCalculations do
       # Remaining work is rounded to 0.06h for all values between 0.0550h and
       # 0.0649h. If user enters values from 75% to 77% while work is 0.25h, then
       # derived remaining work is always 0.06h, so all those % complete values
-      # must be considered consistent progress values.
+      # must be considered ok and are not to be corrected.
       75.upto(77) do |percent_complete|
-        expect(work: 0.25, remaining_work: 0.06, percent_complete:).to be_consistent_progress_values
+        expect(work: 0.25, remaining_work: 0.06, percent_complete:).not_to have_correctable_percent_complete_value
       end
 
       # Due to floating point precision, when 78% is used, remaining work is derived to 0.54999999
       # which is rounded to 0.05h, so 78% would not be considered a consistent progress value.
-      expect(work: 0.25, remaining_work: 0.06, percent_complete: 78).not_to be_consistent_progress_values
+      expect(work: 0.25, remaining_work: 0.06, percent_complete: 78).to have_correctable_percent_complete_value
       # but 78% is consistent with the remaining work of 0.05h
-      expect(work: 0.25, remaining_work: 0.05, percent_complete: 78).to be_consistent_progress_values
+      expect(work: 0.25, remaining_work: 0.05, percent_complete: 78).not_to have_correctable_percent_complete_value
     end
 
     it "tolerates a range of remaining work values when work is too large and " \
@@ -198,17 +245,17 @@ RSpec.describe WorkPackages::SetAttributesService::ProgressValuesCalculations do
       # derived % complete will always be 1%, so all those combinations must be
       # considered consistent progress values.
       986.upto(999) do |remaining_work|
-        expect(work: 1000, remaining_work:, percent_complete: 1).to be_consistent_progress_values
+        expect(work: 1000, remaining_work:, percent_complete: 1).not_to have_correctable_percent_complete_value
       end
 
       # tests for remaining work = 999.99h => 1%
-      expect(work: 1000, remaining_work: 999.99, percent_complete: 1).to be_consistent_progress_values
+      expect(work: 1000, remaining_work: 999.99, percent_complete: 1).not_to have_correctable_percent_complete_value
       # tests for remaining work = 1000h => 0%
-      expect(work: 1000, remaining_work: 1000.0, percent_complete: 1).not_to be_consistent_progress_values
-      expect(work: 1000, remaining_work: 1000.0, percent_complete: 0).to be_consistent_progress_values
+      expect(work: 1000, remaining_work: 1000.0, percent_complete: 0).not_to have_correctable_percent_complete_value
+      expect(work: 1000, remaining_work: 1000.0, percent_complete: 1).to have_correctable_percent_complete_value
       # tests for remaining work = 984.99h => 2%
-      expect(work: 1000, remaining_work: 984.99, percent_complete: 1).not_to be_consistent_progress_values
-      expect(work: 1000, remaining_work: 984.99, percent_complete: 2).to be_consistent_progress_values
+      expect(work: 1000, remaining_work: 984.99, percent_complete: 2).not_to have_correctable_percent_complete_value
+      expect(work: 1000, remaining_work: 984.99, percent_complete: 1).to have_correctable_percent_complete_value
     end
 
     it "tolerates a range of remaining work values when work is too large and " \
@@ -220,17 +267,17 @@ RSpec.describe WorkPackages::SetAttributesService::ProgressValuesCalculations do
       # derived % complete will always be 99%, so all those combinations must be
       # considered consistent progress values.
       1.upto(15) do |remaining_work|
-        expect(work: 1000, remaining_work:, percent_complete: 99).to be_consistent_progress_values
+        expect(work: 1000, remaining_work:, percent_complete: 99).not_to have_correctable_percent_complete_value
       end
 
       # tests for remaining work = 0.01h => 99%
-      expect(work: 1000, remaining_work: 0.01, percent_complete: 99).to be_consistent_progress_values
+      expect(work: 1000, remaining_work: 0.01, percent_complete: 99).not_to have_correctable_percent_complete_value
       # tests for remaining work = 0h => 100%
-      expect(work: 1000, remaining_work: 0.0, percent_complete: 99).not_to be_consistent_progress_values
-      expect(work: 1000, remaining_work: 0.0, percent_complete: 100).to be_consistent_progress_values
+      expect(work: 1000, remaining_work: 0.0, percent_complete: 100).not_to have_correctable_percent_complete_value
+      expect(work: 1000, remaining_work: 0.0, percent_complete: 99).to have_correctable_percent_complete_value
       # tests for remaining work = 16h => 98%
-      expect(work: 1000, remaining_work: 16, percent_complete: 99).not_to be_consistent_progress_values
-      expect(work: 1000, remaining_work: 16, percent_complete: 98).to be_consistent_progress_values
+      expect(work: 1000, remaining_work: 16, percent_complete: 98).not_to have_correctable_percent_complete_value
+      expect(work: 1000, remaining_work: 16, percent_complete: 99).to have_correctable_percent_complete_value
     end
   end
 end

spec/services/work_packages/set_attributes_service_spec.rb

@@ -143,8 +143,8 @@ RSpec.describe WorkPackages::SetAttributesService,
 
   # Scenarios specified in https://community.openproject.org/wp/40749
   # Just checking that everything is correctly wired up. All other scenarios tested in:
-  # - spec/services/work_packages/set_attributes_service/update_progress_values_status_based_spec.rb
-  # - spec/services/work_packages/set_attributes_service/update_progress_values_work_based_spec.rb
+  # - spec/services/work_packages/set_attributes_service/derive_progress_values_status_based_spec.rb
+  # - spec/services/work_packages/set_attributes_service/derive_progress_values_work_based_spec.rb
   describe "deriving progress values attributes" do
     context "in status-based mode",
             with_settings: { work_package_done_ratio: "status" } do