Make done_ratio nil by default

.rubocop.yml

@@ -257,8 +257,10 @@ RSpec/ContextWording:
     - as
     - even
     - for
+    - given
     - having
     - if
+    - in
     - 'on'
     - to
     - unless

app/models/work_package/validations.rb

@@ -33,7 +33,7 @@ module WorkPackage::Validations
     validates :subject, :priority, :project, :type, :author, :status, presence: true
 
     validates :subject, length: { maximum: 255 }
-    validates :done_ratio, inclusion: { in: 0..100 }
+    validates :done_ratio, inclusion: { in: 0..100 }, allow_nil: true
     validates :estimated_hours, numericality: { allow_nil: true }
     validates :remaining_hours, numericality: { allow_nil: true, greater_than_or_equal_to: 0 }
     validates :derived_remaining_hours, numericality: { allow_nil: true, greater_than_or_equal_to: 0 }

config/initializers/new_framework_defaults_7_0.rb

@@ -90,7 +90,7 @@
 # This default means that all columns will be referenced in INSERT queries
 # regardless of whether they have a default or not.
 # Previous versions had true. Rails 7.0+ default is false.
-# Rails.application.config.active_record.partial_inserts = false
+Rails.application.config.active_record.partial_inserts = false
 
 # https://guides.rubyonrails.org/configuring.html#config-action-controller-raise-on-open-redirects
 # Protect from open redirect attacks in `redirect_back_or_to` and `redirect_to`.

db/migrate/20240307094432_change_done_ratio_default_value_to_null.rb

@@ -0,0 +1,8 @@
+class ChangeDoneRatioDefaultValueToNull < ActiveRecord::Migration[7.1]
+  def change
+    change_column_null :work_packages, :done_ratio, true
+    change_column_default :work_packages, :done_ratio, from: 0, to: nil
+    change_column_null :work_package_journals, :done_ratio, true
+    change_column_default :work_package_journals, :done_ratio, from: 0, to: nil
+  end
+end

spec/models/mail_handler_spec.rb

@@ -1088,7 +1088,7 @@
             .to be_nil
 
           expect(subject.done_ratio)
-            .to be 0
+            .to be_nil
 
           expect(subject.priority)
             .to eql priority_low

spec/models/work_package_spec.rb

@@ -333,77 +333,57 @@ def stub_shared_versions(v = nil)
              is_closed: false,
              default_done_ratio: 0)
     end
-    shared_let(:work_package1) do
+    shared_let(:work_package_new) do
       create(:work_package,
              status: status_new)
     end
-    shared_let(:work_package2) do
+    shared_let(:work_package_assigned) do
       create(:work_package,
-             project: work_package1.project,
+             project: work_package_new.project,
              status: status_assigned,
              done_ratio: 30)
     end
 
     describe '#value' do
-      context 'work package field' do
-        before { allow(Setting).to receive(:work_package_done_ratio).and_return 'field' }
-
-        context 'work package 1' do
-          subject { work_package1.done_ratio }
-
-          it { is_expected.to eq(0) }
-        end
-
-        context 'work package 2' do
-          subject { work_package2.done_ratio }
-
-          it { is_expected.to eq(30) }
+      context 'for work-based mode',
+              with_settings: { work_package_done_ratio: 'field' } do
+        it 'returns the value from work package field' do
+          expect(work_package_new.done_ratio).to be_nil
+          expect(work_package_assigned.done_ratio).to eq(30)
         end
       end
 
-      context 'work package status' do
-        before { allow(Setting).to receive(:work_package_done_ratio).and_return 'status' }
-
-        context 'work package 1' do
-          subject { work_package1.done_ratio }
-
-          it { is_expected.to eq(50) }
-        end
-
-        context 'work package 2' do
-          subject { work_package2.done_ratio }
-
-          it { is_expected.to eq(0) }
+      context 'for status-based mode',
+              with_settings: { work_package_done_ratio: 'status' } do
+        it 'uses the % Complete value from the work package status' do
+          expect(work_package_new.done_ratio).to eq(status_new.default_done_ratio)
+          expect(work_package_assigned.done_ratio).to eq(status_assigned.default_done_ratio)
         end
       end
     end
 
     describe '#update_done_ratio_from_status' do
-      context 'work package field' do
-        before do
-          allow(Setting).to receive(:work_package_done_ratio).and_return 'field'
-
-          work_package1.update_done_ratio_from_status
-          work_package2.update_done_ratio_from_status
-        end
-
+      context 'for work-based mode',
+              with_settings: { work_package_done_ratio: 'field' } do
         it 'does not update the done ratio' do
-          expect(work_package1.done_ratio).to eq(0)
-          expect(work_package2.done_ratio).to eq(30)
+          expect { work_package_new.update_done_ratio_from_status }
+            .not_to change { work_package_new[:done_ratio] }
+          expect { work_package_assigned.update_done_ratio_from_status }
+            .not_to change { work_package_assigned[:done_ratio] }
         end
       end
 
-      context 'work package status' do
-        before do
-          allow(Setting).to receive(:work_package_done_ratio).and_return 'status'
-
-          work_package1.update_done_ratio_from_status
-          work_package2.update_done_ratio_from_status
-        end
+      context 'for status-based mode',
+              with_settings: { work_package_done_ratio: 'status' } do
+        it 'updates the done ratio without saving it' do
+          expect { work_package_new.update_done_ratio_from_status }
+            .to change { work_package_new[:done_ratio] }
+            .from(nil).to(50)
+          expect { work_package_assigned.update_done_ratio_from_status }
+            .to change { work_package_assigned[:done_ratio] }
+            .from(30).to(0)
 
-        it 'updates the done ratio' do
-          expect(work_package1.done_ratio).to eq(50)
-          expect(work_package2.done_ratio).to eq(0)
+          expect(work_package_new).to have_changes_to_save
         end
       end
     end

spec/services/work_packages/set_attributes_service_spec.rb

@@ -30,6 +30,10 @@
 
 RSpec.describe WorkPackages::SetAttributesService,
                type: :model do
+  shared_let(:status_no_pct_complete) { create(:status, default_done_ratio: nil, name: 'no % complete') }
+  shared_let(:status_50_pct_complete) { create(:status, default_done_ratio: 50, name: '50% complete') }
+  shared_let(:status_70_pct_complete) { create(:status, default_done_ratio: 70, name: '70% complete') }
+
   let(:today) { Time.zone.today }
   let(:user) { build_stubbed(:user) }
   let(:project) do
@@ -39,7 +43,7 @@
     p
   end
   let(:work_package) do
-    wp = build_stubbed(:work_package, project:)
+    wp = build_stubbed(:work_package, project:, status: status_no_pct_complete)
     wp.type = initial_type
     wp.send(:clear_changes_information)
 
@@ -72,7 +76,7 @@
                         contract_class: mock_contract)
   end
 
-  shared_examples_for 'service call' do
+  shared_examples_for 'service call' do |description: nil|
     subject do
       allow(work_package)
         .to receive(:save)
@@ -84,7 +88,7 @@
       expect(subject).to be_success
     end
 
-    it 'sets the value' do
+    it description || 'sets the value' do
       next if !defined?(expected_attributes) || expected_attributes.blank?
 
       subject
@@ -145,68 +149,112 @@
     it_behaves_like 'service call'
   end
 
-  context 'for estimated_hours' do
-    context 'with the status being used for done_ratio computations',
+  describe 'deriving remaining work attribute (remaining_hours)' do
+    context 'in status-based mode',
             with_settings: { work_package_done_ratio: 'status' } do
-      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) }
-
+      context 'given a work package with work, remaining work, and status with % complete being set' do
         before do
-          work_package.status = status
+          work_package.status = status_50_pct_complete
+          work_package.done_ratio = work_package.status.default_done_ratio
           work_package.estimated_hours = 10.0
           work_package.remaining_hours = 5.0
           work_package.send(:clear_changes_information)
         end
 
-        context 'and estimated_hours are subsequently unset' do
+        context 'when work is unset' do
           let(:call_attributes) { { estimated_hours: nil } }
-          let(:expected_attributes) { { estimated_hours: nil, remaining_hours: nil, done_ratio: 50 } }
+          let(:expected_attributes) { { remaining_hours: nil } }
 
-          it_behaves_like 'service call'
+          it_behaves_like 'service call', description: 'unsets remaining work'
         end
 
-        context 'and estimated_hours are subsequently modified' do
+        context 'when work is modified' do
           let(:call_attributes) { { estimated_hours: 5.0 } }
-          let(:expected_attributes) { { estimated_hours: 5.0, remaining_hours: 2.5, done_ratio: 50 } }
+          let(:expected_attributes) { { remaining_hours: 2.5 } }
 
-          it_behaves_like 'service call'
+          it_behaves_like 'service call', description: 'recomputes remaining work accordingly'
         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 } }
+        context 'when another status with a default % complete value is set' do
+          let(:call_attributes) { { status: status_70_pct_complete } }
+          let(:expected_attributes) { { remaining_hours: 3.0 } }
 
-          before do
-            work_package.status = other_status
-          end
+          it_behaves_like 'service call',
+                          description: 'recomputes remaining work according to the % complete value of the new status'
+        end
+
+        context 'when another status without any default % complete value is set', skip: 'TODO: not implemented yet' do
+          let(:call_attributes) { { status: status_no_pct_complete } }
+          let(:expected_attributes) { { remaining_hours: nil } }
 
-          it_behaves_like 'service call'
+          it_behaves_like 'service call',
+                          description: 'unsets remaining work'
+        end
+      end
+
+      context 'given a work package with work and remaining work unset, and a status with no % complete' do
+        before do
+          work_package.status = status_no_pct_complete
+          work_package.done_ratio = work_package.status.default_done_ratio
+          work_package.estimated_hours = nil
+          work_package.remaining_hours = nil
+          work_package.send(:clear_changes_information)
+        end
+
+        context 'when another status with a default % complete value is set' do
+          let(:call_attributes) { { status: status_70_pct_complete } }
+          let(:expected_attributes) { { remaining_hours: nil } }
+
+          it_behaves_like 'service call',
+                          description: 'remaining work remains unset'
+        end
+
+        context 'when work is set' do
+          let(:call_attributes) { { estimated_hours: 10.0 } }
+          let(:expected_attributes) { { remaining_hours: nil } }
+
+          it_behaves_like 'service call',
+                          description: 'remaining work remains unset'
         end
       end
     end
   end
 
-  context 'for done_ratio' do
-    context 'with the status being used for done_ratio computations',
+  describe 'deriving % complete attribute (done_ratio)' do
+    context 'in status-based mode',
             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 } }
+      context 'given a work package with a status with 50% complete' do
+        before do
+          work_package.status = status_50_pct_complete
+          work_package.done_ratio = work_package.status.default_done_ratio
+          work_package.send(:clear_changes_information)
+        end
 
-      before do
-        work_package.status = status
-      end
+        context 'when another status with another % complete value is set' do
+          let(:call_attributes) { { status: status_70_pct_complete } }
+          let(:expected_attributes) { { done_ratio: 70 } }
 
-      it_behaves_like 'service call'
+          it_behaves_like 'service call', description: 'sets the % complete value to the status default % complete value'
+        end
+
+        context 'when another status with no % complete value is set', skip: 'TODO: not implemented yet' do
+          let(:call_attributes) { { status: status_no_pct_complete } }
+          let(:expected_attributes) { { done_ratio: nil } }
+
+          it_behaves_like 'service call', description: 'unsets the % complete value'
+        end
+      end
     end
 
+    ###
+    ### Below this line, it has not be rewritten / revised yet ###
+    ###
+
     describe 'is "unset"' do
       context 'when estimated_hours is unset' do
         let(:call_attributes) { { estimated_hours: nil } }
         let(:expected_attributes) do
-          { estimated_hours: nil, done_ratio: 0 }
+          { estimated_hours: nil, done_ratio: nil }
         end
 
         it_behaves_like 'service call'
@@ -215,7 +263,7 @@
       context 'when remaining_hours is unset' do
         let(:call_attributes) { { remaining_hours: nil } }
         let(:expected_attributes) do
-          { remaining_hours: nil, done_ratio: 0 }
+          { remaining_hours: nil, done_ratio: nil }
         end
 
         it_behaves_like 'service call'
@@ -224,7 +272,7 @@
       context 'when both estimated_hours and remaining_hours are unset' do
         let(:call_attributes) { { estimated_hours: nil, remaining_hours: nil } }
         let(:expected_attributes) do
-          { estimated_hours: nil, remaining_hours: nil, done_ratio: 0 }
+          { estimated_hours: nil, remaining_hours: nil, done_ratio: nil }
         end
 
         it_behaves_like 'service call'

spec/services/work_packages/update_service_integration_spec.rb

@@ -495,7 +495,7 @@
       # unchanged
       sibling1_work_package.reload
       expect(sibling1_work_package.done_ratio)
-        .to eq(0)
+        .to be_nil
 
       sibling2_work_package.reload
       expect(sibling2_work_package.done_ratio)