Compute remaining_hours from other fields when in Status-based mode

Rules when using a status-based mode for keeping % Complete in sync
with the other two work fields apply a bit differently. To ensure
the correct order of operations and in-time updates, this commit moves
the necessary updates to the service layer along with the computation
formulas for correctly deriving fields from one another.

app/services/work_packages/set_attributes_service.rb

@@ -63,6 +63,7 @@ def set_calculated_attributes(attributes)
     shift_dates_to_soonest_working_days
     update_duration
     update_done_ratio
+    update_remaining_hours
     update_derivable
     update_project_dependent_attributes
     reassign_invalid_status_if_type_changed
@@ -290,14 +291,21 @@ def update_duration
   # Unless both +remaining_hours+ and +estimated_hours+ are set, +done_ratio+ will be
   # considered 0.
   def update_done_ratio
-    return if WorkPackage.use_status_for_done_ratio?
-    return unless work_package.remaining_hours_changed? || work_package.estimated_hours_changed?
+    if WorkPackage.use_status_for_done_ratio?
+      return unless model.status_id_changed?
 
-    work_package.done_ratio = if done_ratio_dependent_attribute_unset?
-                                0
-                              else
-                                compute_done_ratio
-                              end
+      if model.status&.default_done_ratio
+        model.done_ratio = model.status.default_done_ratio
+      end
+    else
+      return unless work_package.remaining_hours_changed? || work_package.estimated_hours_changed?
+
+      work_package.done_ratio = if done_ratio_dependent_attribute_unset?
+                                  0
+                                else
+                                  compute_done_ratio
+                                end
+    end
   end
 
   def done_ratio_dependent_attribute_unset?
@@ -311,6 +319,26 @@ def compute_done_ratio
     (completion_ratio * 100).round(2)
   end
 
+  # When in "Status-based" mode for % Complete, remaining hours are based
+  # on the computation of it derived from the status's default done ratio
+  # and the estimated hours. If the estimated hours are unset, then also
+  # unset the remaining hours.
+  def update_remaining_hours
+    if WorkPackage.use_status_for_done_ratio? &&
+       model.status &&
+       model.status.default_done_ratio
+      model.remaining_hours = if model.estimated_hours
+                                remaining_hours_from_done_ratio_and_estimated_hours
+                              end
+    end
+  end
+
+  def remaining_hours_from_done_ratio_and_estimated_hours
+    ((((model.done_ratio.to_f / 100) * model.estimated_hours) \
+      - model.estimated_hours) \
+      * -1).abs
+  end
+
   def set_version_to_nil
     if work_package.version &&
        work_package.project &&

app/services/work_packages/update_ancestors_service.rb

@@ -90,12 +90,18 @@ def derive_attributes(work_package, loader, attributes)
       # Derived estimated hours and Derived remaining hours need to be
       # calculated before the Derived done ratio below since the
       # aggregation depends on both derived fields.
-      #
       # Changes in any of these, also warrant a recalculation of
       # the Derived done ratio.
+      #
+      # Changes to estimated hours also warrant a recalculation of
+      # derived done ratios in the work package's ancestry as the
+      # derived estimated hours would affect the derived done ratio
+      # or the derived remaining hours, depending on the % Complete mode
+      # currently active.
+      #
       %i[estimated_hours] => :derive_estimated_hours,
       %i[remaining_hours] => :derive_remaining_hours,
-      %i[done_ratio status status_id] => :derive_done_ratio,
+      %i[estimated_hours done_ratio status status_id] => :derive_done_ratio,
       %i[ignore_non_working_days] => :derive_ignore_non_working_days
     }.each do |derivative_attributes, method|
       if attributes.intersect?(derivative_attributes + %i[parent parent_id])
@@ -114,8 +120,6 @@ def derive_done_ratio(ancestor, loader)
     return if initiator?(ancestor)
     return if WorkPackage.done_ratio_disabled?
 
-    return if WorkPackage.use_status_for_done_ratio? && ancestor.status && ancestor.status.default_done_ratio
-
     ancestor.derived_done_ratio = compute_derived_done_ratio(ancestor, loader) || 0
   end
 

spec/services/work_packages/set_attributes_service_spec.rb

@@ -145,27 +145,61 @@
     it_behaves_like 'service call'
   end
 
-  context 'for done_ratio' do
+  context 'for estimated_hours' do
     context 'with the status being used for done_ratio computations',
             with_settings: { work_package_done_ratio: 'status' } do
-      let!(:status) { create(:status, default_done_ratio: 99) }
-      let(:call_attributes) { { estimated_hours: 10.0, remaining_hours: 5.0 } }
-      let(:expected_attributes) { { estimated_hours: 10.0, remaining_hours: 5.0, done_ratio: 99 } }
+      context 'when the work package has estimated hours and remaining hours set along with a done ratio' do
+        let!(:status) { create(:status, default_done_ratio: 50) }
 
-      before do
-        allow(work_package).to receive(:done_ratio=) # Spying-purposes
+        before do
+          work_package.status = status
+          work_package.estimated_hours = 10.0
+          work_package.remaining_hours = 5.0
+          work_package.send(:clear_changes_information)
+        end
 
-        work_package.status = status
-      end
+        context 'and estimated_hours are subsequently unset' do
+          let(:call_attributes) { { estimated_hours: nil } }
+          let(:expected_attributes) { { estimated_hours: nil, remaining_hours: nil, done_ratio: 50 } }
 
-      it_behaves_like 'service call' do
-        it 'does not try to compute and assign done_ratio' do
-          subject
+          it_behaves_like 'service call'
+        end
 
-          expect(work_package)
-            .not_to have_received(:done_ratio=)
+        context 'and estimated_hours are subsequently modified' do
+          let(:call_attributes) { { estimated_hours: 5.0 } }
+          let(:expected_attributes) { { estimated_hours: 5.0, remaining_hours: 2.5, done_ratio: 50 } }
+
+          it_behaves_like 'service call'
         end
+
+        context 'and the status is subsequently changed' do
+          let!(:other_status) { create(:status, default_done_ratio: 70) }
+          let(:call_attributes) { {} }
+          let(:expected_attributes) { { estimated_hours: 10.0, remaining_hours: 3.0 } }
+
+          before do
+            work_package.status = other_status
+          end
+
+          it_behaves_like 'service call'
+        end
+      end
+    end
+  end
+
+  context 'for done_ratio' do
+    context 'with the status being used for done_ratio computations',
+            with_settings: { work_package_done_ratio: 'status' } do
+      let!(:status) { create(:status, default_done_ratio: 50) }
+      let(:call_attributes) { { estimated_hours: 10.0 } }
+      # remaining hours computed from estimated hours and the status's default done ratio
+      let(:expected_attributes) { { estimated_hours: 10.0, remaining_hours: 5.0, done_ratio: 50 } }
+
+      before do
+        work_package.status = status
       end
+
+      it_behaves_like 'service call'
     end
 
     describe 'is "unset"' do