Merge pull request #15213 from opf/feature-add-turbo-frame-morphing

[#54275] Live update the work package progress modal with morphing on turbo frame re-rendering.

app/components/work_packages/progress/base_modal_component.rb

@@ -36,21 +36,26 @@ class BaseModalComponent < ApplicationComponent
 
       FIELD_MAP = {
         "estimatedTime" => :estimated_hours,
-        "remainingTime" => :remaining_hours
+        "work_package[estimated_hours]" => :estimated_hours,
+        "remainingTime" => :remaining_hours,
+        "work_package[remaining_hours]" => :remaining_hours,
+        "work_package[status_id]" => :status_id,
+        "statusId" => :status_id
       }.freeze
 
       include ApplicationHelper
       include Turbo::FramesHelper
       include OpPrimer::ComponentHelpers
       include OpenProject::StaticRouting::UrlHelpers
 
-      attr_reader :work_package, :mode, :focused_field
+      attr_reader :work_package, :mode, :focused_field, :touched_field_map
 
-      def initialize(work_package, focused_field: nil)
+      def initialize(work_package, focused_field: nil, touched_field_map: {})
         super()
 
         @work_package = work_package
         @focused_field = map_field(focused_field)
+        @touched_field_map = touched_field_map
       end
 
       def submit_path

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

@@ -2,13 +2,18 @@
   <%= primer_form_with(
         model: work_package,
         url: submit_path,
-        class: 'progress-form',
-        id: 'progress-form',
-        data: { 'application-target': "dynamic",
-                controller: "work-packages--progress--focus-field" }) do |f| %>
+        class: "progress-form",
+        id: "progress-form",
+        html: { autocomplete: "off" },
+        data: { "application-target": "dynamic",
+                "work-packages--progress--preview-progress-target": "form",
+                controller: "work-packages--progress--focus-field " \
+                            "work-packages--progress--preview-progress " \
+                            "work-packages--progress--touched-field-marker" }
+      ) do |f| %>
     <%= flex_layout do |modal_body| %>
       <% modal_body.with_row(classes: "FormControl-horizontalGroup--sm-vertical") do |_fields| %>
-        <%= render(WorkPackages::ProgressForm.new(f, work_package:, mode:, focused_field:)) %>
+        <%= render(WorkPackages::ProgressForm.new(f, work_package:, mode:, focused_field:, touched_field_map:)) %>
       <% end %>
 
       <% modal_body.with_row(mt: 3) do |_tooltip| %>

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

@@ -32,7 +32,7 @@ module WorkPackages
   module Progress
     module StatusBased
       class ModalBodyComponent < BaseModalComponent # rubocop:disable OpenProject/AddPreviewForViewComponent
-        def initialize(work_package, focused_field: nil)
+        def initialize(work_package, focused_field: nil, touched_field_map: {})
           super
 
           @mode = :status_based

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

@@ -4,11 +4,16 @@
         url: submit_path,
         class: "progress-form",
         id: "progress-form",
+        html: { autocomplete: "off" },
         data: { "application-target": "dynamic",
-                controller: "work-packages--progress--focus-field" }) do |f| %>
+                "work-packages--progress--preview-progress-target": "form",
+                controller: "work-packages--progress--focus-field " \
+                            "work-packages--progress--preview-progress " \
+                            "work-packages--progress--touched-field-marker" }
+      ) do |f| %>
     <%= flex_layout do |modal_body| %>
       <% modal_body.with_row(classes: "FormControl-horizontalGroup--sm-vertical") do |_fields| %>
-        <%= render(WorkPackages::ProgressForm.new(f, work_package:, mode:, focused_field:)) %>
+        <%= render(WorkPackages::ProgressForm.new(f, work_package:, mode:, focused_field:, touched_field_map:)) %>
       <% end %>
 
       <% if should_display_migration_warning? %>

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

@@ -33,7 +33,7 @@ module Progress
     module WorkBased
       # rubocop:disable OpenProject/AddPreviewForViewComponent
       class ModalBodyComponent < BaseModalComponent
-        def initialize(work_package, focused_field: nil)
+        def initialize(work_package, focused_field: nil, touched_field_map: {})
           super
 
           @mode = :work_based

app/components/work_packages/share/permission_button_component.rb

@@ -72,7 +72,7 @@ def active_role
       end
 
       def permission_name(value)
-        options.select { |option| option[:value] == value }
+        options.find { |option| option[:value] == value }[:label]
       end
 
       def form_inputs(role_id)

app/controllers/work_packages/progress_controller.rb

@@ -36,17 +36,28 @@ class WorkPackages::ProgressController < ApplicationController
 
   layout false
   before_action :set_work_package
+  before_action :extract_persisted_progress_attributes, only: %i[edit create update]
 
   helper_method :modal_class
 
+  def new
+    build_up_brand_new_work_package
+
+    render modal_class.new(@work_package,
+                           focused_field: params[:field],
+                           touched_field_map:)
+  end
+
   def edit
-    build_up_new_work_package
+    build_up_work_package
 
-    render modal_class.new(@work_package, focused_field: params[:field])
+    render modal_class.new(@work_package,
+                           focused_field: params[:field],
+                           touched_field_map:)
   end
 
   def create
-    service_call = build_up_new_work_package
+    service_call = build_up_brand_new_work_package
 
     if service_call.errors
                    .map(&:attribute)
@@ -70,7 +81,7 @@ def update
     service_call = WorkPackages::UpdateService
                      .new(user: current_user,
                           model: @work_package)
-                     .call(update_work_package_params)
+                     .call(work_package_params)
 
     if service_call.success?
       respond_to do |format|
@@ -105,13 +116,19 @@ def set_work_package
     @work_package = WorkPackage.new
   end
 
-  def create_work_package_params
-    params.require(:work_package)
-          .permit(allowed_params)
-          .compact_blank
+  def touched_field_map
+    params.require(:work_package).permit("estimated_hours_touched",
+                                         "remaining_hours_touched",
+                                         "status_id_touched").to_h
+  end
+
+  def extract_persisted_progress_attributes
+    @persisted_progress_attributes = @work_package
+                                       .attributes
+                                       .slice("estimated_hours", "remaining_hours", "status_id")
   end
 
-  def update_work_package_params
+  def work_package_params
     params.require(:work_package)
           .permit(allowed_params)
   end
@@ -124,12 +141,46 @@ def allowed_params
     end
   end
 
-  def build_up_new_work_package
+  def reject_params_that_dont_differ_from_persisted_values
+    work_package_params.reject do |key, value|
+      @persisted_progress_attributes[key.to_s].to_f.to_s == value.to_f.to_s
+    end
+  end
+
+  def filtered_work_package_params
+    {}.tap do |filtered_params|
+      filtered_params[:estimated_hours] = work_package_params["estimated_hours"] if estimated_hours_touched?
+      filtered_params[:remaining_hours] = work_package_params["remaining_hours"] if remaining_hours_touched?
+      filtered_params[:status_id] = work_package_params["status_id"] if status_id_touched?
+    end
+  end
+
+  def estimated_hours_touched?
+    params.require(:work_package)[:estimated_hours_touched] == "true"
+  end
+
+  def remaining_hours_touched?
+    params.require(:work_package)[:remaining_hours_touched] == "true"
+  end
+
+  def status_id_touched?
+    params.require(:work_package)[:status_id_touched] == "true"
+  end
+
+  def build_up_work_package
+    WorkPackages::SetAttributesService
+      .new(user: current_user,
+           model: @work_package,
+           contract_class: WorkPackages::CreateContract)
+      .call(filtered_work_package_params)
+  end
+
+  def build_up_brand_new_work_package
     WorkPackages::SetAttributesService
       .new(user: current_user,
            model: @work_package,
            contract_class: WorkPackages::CreateContract)
-      .call(create_work_package_params)
+      .call(work_package_params)
   end
 
   def formatted_duration(hours)

app/forms/work_packages/progress_form.rb

@@ -27,14 +27,19 @@
 # ++
 
 class WorkPackages::ProgressForm < ApplicationForm
+  attr_reader :work_package
+
   def initialize(work_package:,
                  mode: :work_based,
-                 focused_field: :remaining_hours)
+                 focused_field: :remaining_hours,
+                 touched_field_map: {})
     super()
 
     @work_package = work_package
     @mode = mode
-    @focused_field = focused_field_by_selection(focused_field) || focused_field_by_error
+    @focused_field = focused_field_by_selection(focused_field)
+    @touched_field_map = touched_field_map
+    ensure_only_one_error_for_remaining_work_exceeding_work
   end
 
   form do |query_form|
@@ -58,17 +63,51 @@ def initialize(work_package:,
 
         render_text_field(group, name: :estimated_hours, label: I18n.t(:label_work))
         render_readonly_text_field(group, name: :remaining_hours, label: I18n.t(:label_remaining_work))
+
+        # Add a hidden field in create forms as the select field is disabled and is otherwise not included in the form payload
+        group.hidden(name: :status_id) if @work_package.new_record?
+
+        group.hidden(name: :status_id_touched,
+                     value: @touched_field_map["status_id_touched"] || false,
+                     data: { "work-packages--progress--touched-field-marker-target": "touchedFieldInput",
+                             "referrer-field": "work_package[status_id]" })
+        group.hidden(name: :estimated_hours_touched,
+                     value: @touched_field_map["estimated_hours_touched"] || false,
+                     data: { "work-packages--progress--touched-field-marker-target": "touchedFieldInput",
+                             "referrer-field": "work_package[estimated_hours]" })
       else
         render_text_field(group, name: :estimated_hours, label: I18n.t(:label_work))
         render_text_field(group, name: :remaining_hours, label: I18n.t(:label_remaining_work),
                                  disabled: disabled_remaining_work_field?)
         render_readonly_text_field(group, name: :done_ratio, label: I18n.t(:label_percent_complete))
+
+        group.hidden(name: :estimated_hours_touched,
+                     value: @touched_field_map["estimated_hours_touched"] || false,
+                     data: { "work-packages--progress--touched-field-marker-target": "touchedFieldInput",
+                             "referrer-field": "work_package[estimated_hours]" })
+        group.hidden(name: :remaining_hours_touched,
+                     value: @touched_field_map["remaining_hours_touched"] || false,
+                     data: { "work-packages--progress--touched-field-marker-target": "touchedFieldInput",
+                             "referrer-field": "work_package[remaining_hours]" })
       end
     end
   end
 
   private
 
+  def ensure_only_one_error_for_remaining_work_exceeding_work
+    if work_package.errors.added?(:remaining_hours, :cant_exceed_work) &&
+       work_package.errors.added?(:estimated_hours, :cant_be_inferior_to_remaining_work)
+      error_to_delete =
+        if @focused_field == :estimated_hours
+          :remaining_hours
+        else
+          :estimated_hours
+        end
+      work_package.errors.delete(error_to_delete)
+    end
+  end
+
   def focused_field_by_selection(field)
     if field == :remaining_hours && @work_package.estimated_hours.nil?
       :estimated_hours
@@ -77,24 +116,6 @@ def focused_field_by_selection(field)
     end
   end
 
-  # First field with an error is focused. If it's readonly or disabled, then the
-  # field before it will be focused
-  def focused_field_by_error
-    fields = if @mode == :work_based
-               %i[estimated_hours remaining_hours done_ratio]
-             else
-               %i[status_id estimated_hours remaining_hours]
-             end
-
-    fields.each do |field_name|
-      break if @focused_field
-
-      @focused_field = field_name if @work_package.errors.map(&:attribute).include?(field_name)
-    end
-
-    @focused_field
-  end
-
   def render_text_field(group,
                         name:,
                         label:,
@@ -147,11 +168,13 @@ def format_to_smallest_fractional_part(number)
   end
 
   def default_field_options(name)
+    data = { "work-packages--progress--preview-progress-target": "progressInput",
+             "work-packages--progress--touched-field-marker-target": "progressInput",
+             action: "input->work-packages--progress--touched-field-marker#markFieldAsTouched" }
     if @focused_field == name
-      { data: { "work-packages--progress--focus-field-target": "fieldToFocus" } }
-    else
-      {}
+      data[:"work-packages--progress--focus-field-target"] = "fieldToFocus"
     end
+    { data: }
   end
 
   # Remaining work field is enabled when work is set, or when there are errors

app/services/work_packages/set_attributes_service.rb

@@ -151,6 +151,7 @@ def update_derivable
       work_package.start_date = days.start_date(work_package.due_date, work_package.duration)
     end
   end
+
   # rubocop:enable Metrics/AbcSize
 
   def set_default_attributes(attributes)
@@ -343,6 +344,7 @@ def round_progress_values
   end
 
   def update_remaining_hours_from_percent_complete
+    return if work_package.remaining_hours_came_from_user?
     return if work_package.estimated_hours&.negative?
 
     work_package.remaining_hours = remaining_hours_from_done_ratio_and_estimated_hours
@@ -376,7 +378,7 @@ def invalid_progress_values?
 
   def update_estimated_hours
     return unless WorkPackage.use_field_for_done_ratio?
-    return if work_package.estimated_hours_changed?
+    return if work_package.estimated_hours_came_from_user?
     return unless work_package.remaining_hours_changed?
 
     work = work_package.estimated_hours
@@ -396,8 +398,8 @@ def update_remaining_hours
     if WorkPackage.use_status_for_done_ratio?
       update_remaining_hours_from_percent_complete
     elsif WorkPackage.use_field_for_done_ratio? &&
-          work_package.estimated_hours_changed?
-      return if work_package.remaining_hours_changed?
+      work_package.estimated_hours_changed?
+      return if work_package.remaining_hours_came_from_user?
       return if work_package.estimated_hours&.negative?
       return if work_was_unset_and_remaining_work_is_set? # remaining work is kept and % complete will be set
 
@@ -426,8 +428,8 @@ def remaining_hours_from_done_ratio_and_estimated_hours
 
   def set_version_to_nil
     if work_package.version &&
-       work_package.project &&
-       work_package.project.shared_versions.exclude?(work_package.version)
+      work_package.project &&
+      work_package.project.shared_versions.exclude?(work_package.version)
       work_package.version = nil
     end
   end
@@ -498,7 +500,7 @@ def new_start_date
 
   def new_start_date_from_parent
     return unless work_package.parent_id_changed? &&
-                  work_package.parent
+      work_package.parent
 
     work_package.parent.soonest_start
   end

config/routes.rb

@@ -523,7 +523,7 @@
       end
     end
 
-    resource :progress, only: %i[edit update], controller: "work_packages/progress"
+    resource :progress, only: %i[new edit update], controller: "work_packages/progress"
     collection do
       resource :progress,
                only: :create,