Merge pull request #16041 from opf/feature/52233-allow-to-edit-p-comp…

…lete-in-work-based-mode

[52233] Allow % Complete edition in work-based mode

Gemfile

@@ -275,7 +275,7 @@ group :test do
   gem "selenium-devtools"
   gem "selenium-webdriver", "~> 4.20"
 
-  gem "fuubar", "~> 2.5.0"
+  gem "fuubar", "~> 2.5.0", require: false
   gem "timecop", "~> 0.9.0"
 
   # Record your test suite's HTTP interactions and replay them during future test runs for fast, deterministic, accurate tests.
@@ -322,6 +322,9 @@ group :development, :test do
   gem "ruby-prof", require: false
   gem "stackprof", require: false
 
+  # Output a stack trace anytime, useful when a process is stuck
+  gem "rbtrace"
+
   # REPL with debug commands
   gem "debug"
 
@@ -348,7 +351,7 @@ group :development, :test do
   gem "brakeman", "~> 6.1.0"
 
   # i18n-tasks helps find and manage missing and unused translations.
-  gem "i18n-tasks", "~> 1.0.13"
+  gem "i18n-tasks", "~> 1.0.13", require: false
 end
 
 gem "bootsnap", "~> 1.18.0", require: false

Gemfile.lock

@@ -790,6 +790,7 @@ GEM
     openssl (3.2.0)
     openssl-signature_algorithm (1.3.0)
       openssl (> 2.0)
+    optimist (3.1.0)
     os (1.1.4)
     ox (2.14.18)
     paper_trail (15.1.0)
@@ -921,6 +922,10 @@ GEM
     rb-inotify (0.11.1)
       ffi (~> 1.0)
     rb_sys (0.9.99)
+    rbtrace (0.5.1)
+      ffi (>= 1.0.6)
+      msgpack (>= 0.4.3)
+      optimist (>= 3.0.0)
     rbtree3 (0.7.1)
     rdoc (6.7.0)
       psych (>= 4.0.0)
@@ -1307,6 +1312,7 @@ DEPENDENCIES
   rails (~> 7.1.3)
   rails-controller-testing (~> 1.0.2)
   rails-i18n (~> 7.0.0)
+  rbtrace
   rdoc (>= 2.4.2)
   redis (~> 5.2.0)
   request_store (~> 1.7.0)

app/components/work_packages/progress/status_based/modal_body_component.html.erb

@@ -13,11 +13,20 @@
       ) do |f| %>
     <%= flex_layout do |modal_body| %>
       <% modal_body.with_row(classes: "FormControl-horizontalGroup--sm-vertical") do |_fields| %>
+        <% if OpenProject::FeatureDecisions.percent_complete_edition_active? %>
         <%= render(WorkPackages::ProgressForm.new(f,
                                                   work_package:,
                                                   mode:,
                                                   focused_field:,
                                                   touched_field_map:)) %>
+        <% else %>
+        <%# This condition branch to be removed in 15.0 with :percent_complete_edition feature flag removal %>
+        <%= render(WorkPackages::Pre144ProgressForm.new(f,
+                                                        work_package:,
+                                                        mode:,
+                                                        focused_field:,
+                                                        touched_field_map:)) %>
+        <% end %>
       <% end %>
 
       <% modal_body.with_row(mt: 3) do |_tooltip| %>

app/components/work_packages/progress/work_based/modal_body_component.html.erb

@@ -13,16 +13,28 @@
       ) do |f| %>
     <%= flex_layout do |modal_body| %>
       <% modal_body.with_row(classes: "FormControl-horizontalGroup--sm-vertical") do |_fields| %>
+        <% if OpenProject::FeatureDecisions.percent_complete_edition_active? %>
         <%= render(WorkPackages::ProgressForm.new(f,
                                                   work_package:,
                                                   mode:,
                                                   focused_field:,
                                                   touched_field_map:)) %>
+        <% else %>
+          <%# This condition branch to be removed in 15.0 with :percent_complete_edition feature flag removal %>
+          <%= render(WorkPackages::Pre144ProgressForm.new(f,
+                                                          work_package:,
+                                                          mode:,
+                                                          focused_field:,
+                                                          touched_field_map:)) %>
+        <% end %>
       <% end %>
 
-      <% if should_display_migration_warning? %>
-        <% modal_body.with_row(mt: 3) do |_migration_warning| %>
-          <%= render(Primer::Alpha::Banner.new) { t("work_package.progress.modal.migration_warning_text") } %>
+      <% if !OpenProject::FeatureDecisions.percent_complete_edition_active? %>
+        <%# This condition branch to be removed in 15.0 with :percent_complete_edition feature flag removal %>
+        <% if should_display_migration_warning? %>
+          <% modal_body.with_row(mt: 3) do |_migration_warning| %>
+            <%= render(Primer::Alpha::Banner.new) { t("work_package.progress.modal.migration_warning_text") } %>
+          <% end %>
         <% end %>
       <% end %>
 

app/components/work_packages/progress/work_based/modal_body_component.rb

@@ -41,11 +41,15 @@ def initialize(work_package,
           @mode = :work_based
         end
 
+        # This method can be safely deleted once the feature flag
+        # :percent_complete_edition is removed, which should happen for
+        # OpenProject 15.0 release.
         def should_display_migration_warning?
+          return false if OpenProject::FeatureDecisions.percent_complete_edition_active?
+
           work_package.done_ratio.present? && work_package.estimated_hours.nil? && work_package.remaining_hours.nil?
         end
       end
-
       # rubocop:enable OpenProject/AddPreviewForViewComponent
     end
   end

app/contracts/work_packages/base_contract.rb

@@ -53,19 +53,38 @@ class BaseContract < ::ModelContract
     attribute :project_id
 
     attribute :done_ratio,
-              writable: false
+              writable: ->(*) {
+                          OpenProject::FeatureDecisions.percent_complete_edition_active? \
+                            && WorkPackage.work_based_mode?
+                        } do
+      if OpenProject::FeatureDecisions.percent_complete_edition_active?
+        validate_percent_complete_matches_work_and_remaining_work
+        validate_percent_complete_is_unset_when_work_is_zero
+        validate_percent_complete_is_set_when_work_and_remaining_work_are_set
+      end
+    end
     attribute :derived_done_ratio,
               writable: false
 
     attribute :estimated_hours do
-      validate_work_is_set_when_remaining_work_is_set
+      if OpenProject::FeatureDecisions.percent_complete_edition_active?
+        validate_work_is_set_when_remaining_work_and_percent_complete_are_set
+      else
+        # to be removed in 15.0 with :percent_complete_edition feature flag removal
+        validate_work_is_set_when_remaining_work_is_set
+      end
     end
     attribute :derived_estimated_hours,
               writable: false
 
     attribute :remaining_hours do
       validate_remaining_work_is_lower_than_work
-      validate_remaining_work_is_set_when_work_is_set
+      if OpenProject::FeatureDecisions.percent_complete_edition_active?
+        validate_remaining_work_is_set_when_work_and_percent_complete_are_set
+      else
+        # to be removed in 15.0 with :percent_complete_edition feature flag removal
+        validate_remaining_work_is_set_when_work_is_set
+      end
     end
     attribute :derived_remaining_hours,
               writable: false
@@ -137,7 +156,6 @@ class BaseContract < ::ModelContract
     validate :validate_priority_exists
 
     validate :validate_category
-    validate :validate_estimated_hours
 
     validate :validate_assigned_to_exists
 
@@ -218,12 +236,6 @@ def assignable_assignees
 
     attr_reader :can
 
-    def validate_estimated_hours
-      if !model.estimated_hours.nil? && model.estimated_hours < 0
-        errors.add :estimated_hours, :only_values_greater_or_equal_zeroes_allowed
-      end
-    end
-
     def validate_after_soonest_start(date_attribute)
       if !model.schedule_manually? && before_soonest_start?(date_attribute)
         message = I18n.t("activerecord.errors.models.work_package.attributes.start_date.violates_relationships",
@@ -324,7 +336,7 @@ def validate_version_is_assignable
     end
 
     def validate_remaining_work_is_lower_than_work
-      if work_set? && remaining_work_set? && remaining_work_exceeds_work?
+      if remaining_work_exceeds_work?
         if model.changed.include?("estimated_hours")
           errors.add(:estimated_hours, :cant_be_inferior_to_remaining_work)
         end
@@ -335,28 +347,111 @@ def validate_remaining_work_is_lower_than_work
       end
     end
 
+    # to be removed in 15.0 with :percent_complete_edition feature flag removal
     def validate_remaining_work_is_set_when_work_is_set
       if work_set? && !remaining_work_set?
         errors.add(:remaining_hours, :must_be_set_when_work_is_set)
       end
     end
 
+    # to be removed in 15.0 with :percent_complete_edition feature flag removal
     def validate_work_is_set_when_remaining_work_is_set
       if remaining_work_set? && !work_set?
         errors.add(:estimated_hours, :must_be_set_when_remaining_work_is_set)
       end
     end
 
+    def validate_work_is_set_when_remaining_work_and_percent_complete_are_set
+      if remaining_work_set_and_valid? && percent_complete_set_and_valid? && work_unset?
+        errors.add(:estimated_hours, :must_be_set_when_remaining_work_and_percent_complete_are_set)
+      end
+    end
+
+    def validate_remaining_work_is_set_when_work_and_percent_complete_are_set
+      if work_set_and_valid? && percent_complete_set_and_valid? && remaining_work_unset?
+        errors.add(:remaining_hours, :must_be_set_when_work_and_percent_complete_are_set)
+      end
+    end
+
+    def validate_percent_complete_is_set_when_work_and_remaining_work_are_set
+      if work_set_and_valid? && remaining_work_set_and_valid? && work != 0 && percent_complete_unset?
+        errors.add(:done_ratio, :must_be_set_when_work_and_remaining_work_are_set)
+      end
+    end
+
+    # rubocop:disable Metrics/AbcSize
+    def validate_percent_complete_matches_work_and_remaining_work
+      return if WorkPackage.status_based_mode? || percent_complete_unset? || work == 0
+      return if remaining_work_exceeds_work? # avoid too many error messages at the same time
+      return unless work_set? && remaining_work_set?
+
+      work_done = work - remaining_work
+      expected_percent_complete = (100 * work_done.to_f / work).round
+
+      if percent_complete != expected_percent_complete
+        errors.add(:done_ratio, :does_not_match_work_and_remaining_work)
+      end
+    end
+    # rubocop:enable Metrics/AbcSize
+
+    def validate_percent_complete_is_unset_when_work_is_zero
+      return if WorkPackage.status_based_mode?
+
+      if work == 0 && percent_complete_set?
+        errors.add(:done_ratio, :cannot_be_set_when_work_is_zero)
+      end
+    end
+
+    def work
+      model.estimated_hours
+    end
+
     def work_set?
-      model.estimated_hours.present?
+      work.present?
+    end
+
+    def work_set_and_valid?
+      work_set? && work >= 0
+    end
+
+    def work_unset?
+      work.nil?
+    end
+
+    def remaining_work
+      model.remaining_hours
     end
 
     def remaining_work_set?
-      model.remaining_hours.present?
+      remaining_work.present?
+    end
+
+    def remaining_work_set_and_valid?
+      remaining_work_set? && remaining_work >= 0
+    end
+
+    def remaining_work_unset?
+      remaining_work.nil?
     end
 
     def remaining_work_exceeds_work?
-      model.remaining_hours > model.estimated_hours
+      work_set? && remaining_work_set? && remaining_work > work
+    end
+
+    def percent_complete
+      model.done_ratio
+    end
+
+    def percent_complete_set?
+      percent_complete.present?
+    end
+
+    def percent_complete_set_and_valid?
+      percent_complete_set? && percent_complete.between?(0, 100)
+    end
+
+    def percent_complete_unset?
+      percent_complete.nil?
     end
 
     def validate_no_reopen_on_closed_version