From 1d91460c8481854866a9592336feb6a20baacb03 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Wed, 10 Jun 2026 15:40:15 +0200 Subject: [PATCH] Extract progress modal form glue into a shared ModalParams concern --- .../progress/base_modal_component.rb | 10 +- .../status_based/modal_body_component.rb | 3 +- .../work_based/modal_body_component.rb | 3 +- .../work_packages/progress/modal_params.rb | 125 ++++++++++++++++++ .../work_packages/progress_controller.rb | 78 +---------- 5 files changed, 139 insertions(+), 80 deletions(-) create mode 100644 app/controllers/concerns/work_packages/progress/modal_params.rb diff --git a/app/components/work_packages/progress/base_modal_component.rb b/app/components/work_packages/progress/base_modal_component.rb index 94103d635f1..f24d39bfefc 100644 --- a/app/components/work_packages/progress/base_modal_component.rb +++ b/app/components/work_packages/progress/base_modal_component.rb @@ -59,15 +59,23 @@ module WorkPackages def initialize(work_package, focused_field: nil, - touched_field_map: {}) + touched_field_map: {}, + submit_path: nil) super() @work_package = work_package @focused_field = map_field(focused_field) @touched_field_map = touched_field_map + @submit_path = submit_path end + # Defaults to the core progress controller, but callers reusing the modal + # from another context (e.g. the resource planner) can point the form at + # their own endpoint. The live preview derives its URL from the form + # action too (`/preview`), so a single override covers both. def submit_path + return @submit_path if @submit_path + if work_package.new_record? url_for(controller: "work_packages/progress", action: "create") diff --git a/app/components/work_packages/progress/status_based/modal_body_component.rb b/app/components/work_packages/progress/status_based/modal_body_component.rb index 83bba94fd04..c83c1b36125 100644 --- a/app/components/work_packages/progress/status_based/modal_body_component.rb +++ b/app/components/work_packages/progress/status_based/modal_body_component.rb @@ -34,7 +34,8 @@ module WorkPackages class ModalBodyComponent < BaseModalComponent # rubocop:disable OpenProject/AddPreviewForViewComponent def initialize(work_package, focused_field: nil, - touched_field_map: {}) + touched_field_map: {}, + submit_path: nil) super @mode = :status_based diff --git a/app/components/work_packages/progress/work_based/modal_body_component.rb b/app/components/work_packages/progress/work_based/modal_body_component.rb index f2e66de74fe..54f1607a3b3 100644 --- a/app/components/work_packages/progress/work_based/modal_body_component.rb +++ b/app/components/work_packages/progress/work_based/modal_body_component.rb @@ -35,7 +35,8 @@ module WorkPackages class ModalBodyComponent < BaseModalComponent def initialize(work_package, focused_field: nil, - touched_field_map: {}) + touched_field_map: {}, + submit_path: nil) super @mode = :work_based diff --git a/app/controllers/concerns/work_packages/progress/modal_params.rb b/app/controllers/concerns/work_packages/progress/modal_params.rb new file mode 100644 index 00000000000..f04ca742488 --- /dev/null +++ b/app/controllers/concerns/work_packages/progress/modal_params.rb @@ -0,0 +1,125 @@ +# 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 Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +# ++ + +module WorkPackages + module Progress + # Form glue shared by every controller that renders the progress modal: + # parses the hidden `*_touched` inputs so only fields the user actually + # edited are written, picks the mode-appropriate attributes, and builds the + # modal body component. + module ModalParams + extend ActiveSupport::Concern + + ERROR_PRONE_ATTRIBUTES = %i[status_id + estimated_hours + remaining_hours + done_ratio].freeze + + private + + def progress_modal_component(submit_path: nil) + modal_class.new(@work_package, + focused_field:, + touched_field_map:, + submit_path:) + end + + def modal_class + if WorkPackage.status_based_mode? + WorkPackages::Progress::StatusBased::ModalBodyComponent + else + WorkPackages::Progress::WorkBased::ModalBodyComponent + end + end + + def focused_field + params[:field] + end + + def set_progress_attributes_to_work_package + WorkPackages::SetAttributesService + .new(user: current_user, + model: @work_package, + contract_class:) + .call(work_package_progress_params) + end + + def contract_class + if @work_package.new_record? + WorkPackages::CreateContract + else + WorkPackages::UpdateContract + end + end + + def work_package_progress_params + params.require(:work_package) + .slice(*allowed_touched_params) + .permit! + end + + def allowed_touched_params + allowed_params.filter { touched?(it) } + end + + def allowed_params + if WorkPackage.status_based_mode? + %i[estimated_hours status_id] + else + %i[estimated_hours remaining_hours done_ratio] + end + end + + def touched?(field) + touched_field_map[:"#{field}_touched"] + end + + # Tolerates a missing `work_package` param so the modal can be opened + # without carrying the form values along (e.g. from a context menu). + def touched_field_map + (params[:work_package] || ActionController::Parameters.new) + .slice("estimated_hours_touched", + "remaining_hours_touched", + "done_ratio_touched", + "status_id_touched") + .transform_values { it == "true" } + .permit! + end + + def extra_error_messages(service_call) + errors_not_handled_by_progress_modal = service_call.errors.reject do |error| + ERROR_PRONE_ATTRIBUTES.include?(error.attribute) + end + + join_flash_messages(errors_not_handled_by_progress_modal.map(&:full_message)) + end + end + end +end diff --git a/app/controllers/work_packages/progress_controller.rb b/app/controllers/work_packages/progress_controller.rb index 4c480339f26..4c99457e19a 100644 --- a/app/controllers/work_packages/progress_controller.rb +++ b/app/controllers/work_packages/progress_controller.rb @@ -31,11 +31,7 @@ class WorkPackages::ProgressController < ApplicationController include OpTurbo::ComponentStream include FlashMessagesHelper - - ERROR_PRONE_ATTRIBUTES = %i[status_id - estimated_hours - remaining_hours - done_ratio].freeze + include WorkPackages::Progress::ModalParams layout false authorization_checked! :new, :edit, :preview, :create, :update @@ -132,22 +128,6 @@ class WorkPackages::ProgressController < ApplicationController } end - def progress_modal_component - modal_class.new(@work_package, focused_field:, touched_field_map:) - end - - def modal_class - if WorkPackage.status_based_mode? - WorkPackages::Progress::StatusBased::ModalBodyComponent - else - WorkPackages::Progress::WorkBased::ModalBodyComponent - end - end - - def focused_field - params[:field] - end - def find_work_package @work_package = WorkPackage.visible.find(params[:work_package_id]) end @@ -160,63 +140,7 @@ class WorkPackages::ProgressController < ApplicationController @work_package.clear_changes_information end - def touched_field_map - params.require(:work_package) - .slice("estimated_hours_touched", - "remaining_hours_touched", - "done_ratio_touched", - "status_id_touched") - .transform_values { it == "true" } - .permit! - end - - def work_package_progress_params - params.require(:work_package) - .slice(*allowed_touched_params) - .permit! - end - - def allowed_touched_params - allowed_params.filter { touched?(it) } - end - - def allowed_params - if WorkPackage.status_based_mode? - %i[estimated_hours status_id] - else - %i[estimated_hours remaining_hours done_ratio] - end - end - - def touched?(field) - touched_field_map[:"#{field}_touched"] - end - - def set_progress_attributes_to_work_package - WorkPackages::SetAttributesService - .new(user: current_user, - model: @work_package, - contract_class:) - .call(work_package_progress_params) - end - - def contract_class - if @work_package.new_record? - WorkPackages::CreateContract - else - WorkPackages::UpdateContract - end - end - def formatted_duration(hours) API::V3::Utilities::DateTimeFormatter.format_duration_from_hours(hours, allow_nil: true) end - - def extra_error_messages(service_call) - errors_not_handled_by_progress_modal = service_call.errors.reject do |error| - ERROR_PRONE_ATTRIBUTES.include?(error.attribute) - end - - join_flash_messages(errors_not_handled_by_progress_modal.map(&:full_message)) - end end