diff --git a/app/controllers/work_packages/progress_controller.rb b/app/controllers/work_packages/progress_controller.rb index 4c64000f87fb..e56e7c558d12 100644 --- a/app/controllers/work_packages/progress_controller.rb +++ b/app/controllers/work_packages/progress_controller.rb @@ -133,19 +133,15 @@ def work_package_params .permit(allowed_params).tap do |wp_params| if wp_params["estimated_hours"].present? wp_params["estimated_hours"] = - convert_duration_to_hours(wp_params["estimated_hours"]) + DurationConverter.parse(wp_params["estimated_hours"]) end if wp_params["remaining_hours"].present? wp_params["remaining_hours"] = - convert_duration_to_hours(wp_params["remaining_hours"]) + DurationConverter.parse(wp_params["remaining_hours"]) end end end - def convert_duration_to_hours(duration) - ChronicDuration.parse(duration, keep_zero: true) / 3600.to_f - end - def allowed_params if WorkPackage.use_status_for_done_ratio? %i[estimated_hours status_id] diff --git a/app/forms/work_packages/progress_form.rb b/app/forms/work_packages/progress_form.rb index f74a1ba00d61..0e662e9b177e 100644 --- a/app/forms/work_packages/progress_form.rb +++ b/app/forms/work_packages/progress_form.rb @@ -154,12 +154,12 @@ def render_readonly_text_field(group, def field_value(name) errors = @work_package.errors.where(name) - if user_value = errors.map { |error| error.options[:value] }.find { !_1.nil? } + if (user_value = errors.map { |error| error.options[:value] }.find { !_1.nil? }) user_value elsif name == :done_ratio format_to_smallest_fractional_part(@work_package.public_send(name)) else - format_to_duration(@work_package.public_send(name)) + DurationConverter.output(@work_package.public_send(name)) end end @@ -169,13 +169,6 @@ def format_to_smallest_fractional_part(number) number % 1 == 0 ? number.to_i : number end - def format_to_duration(number) - return number if number.nil? - - # Multiply by 3600 to convert hours to seconds - ChronicDuration.output(number * 3600, keep_zero: true) - end - def default_field_options(name) data = { "work-packages--progress--preview-progress-target": "progressInput", "work-packages--progress--touched-field-marker-target": "progressInput", diff --git a/app/services/duration_converter.rb b/app/services/duration_converter.rb new file mode 100644 index 000000000000..757c49281781 --- /dev/null +++ b/app/services/duration_converter.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2024 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. +# ++ + +# We use BigDecimal to handle floating point arithmetic and avoid +# weird floating point results on decimal operations when converting +# hours to seconds on duration outputting. +require "bigdecimal" + +class DurationConverter + class << self + def parse(duration_string) + # Keep 0 values and convert the extracted duration to hours + # by dividing by 3600. + ChronicDuration.parse(duration_string, keep_zero: true, default_unit: "hours") / 3600.to_f + end + + def output(duration_in_hours) + return duration_in_hours if duration_in_hours.nil? + + duration_in_seconds = convert_duration_to_seconds(duration_in_hours) + + # return "0 h" if parsing 0. + # ChronicDuration returns nil when parsing 0. + # By default, its unit is seconds and if we were + # keeping zeroes, we'd format this as "0 secs". + # + # We want to override this behavior. + if ChronicDuration.output(duration_in_seconds, default_unit: "hours").nil? + "0h" + else + ChronicDuration.output(duration_in_seconds, default_unit: "hours", format: :short) + end + end + + private + + def convert_duration_to_seconds(duration_in_hours) + (BigDecimal(duration_in_hours.to_s) * 3600).to_f + end + end +end diff --git a/spec/services/duration_converter_spec.rb b/spec/services/duration_converter_spec.rb new file mode 100644 index 000000000000..e2346afac081 --- /dev/null +++ b/spec/services/duration_converter_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2024 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. +# ++ + +require "spec_helper" + +RSpec.describe DurationConverter do + describe ".parse" do + it "returns 0 when given 0 duration" do + expect(described_class.parse("0 hrs")).to eq(0) + end + + it "works with ChronicDuration defaults otherwise" do + expect(described_class.parse("5 hrs 30 mins")).to eq(5.5) + end + + it "assumes hours as the default unit for input" do + expect(described_class.parse("5.75")).to eq(5.75) + end + end + + describe ".output" do + it "returns nil when given nil" do + expect(described_class.output(nil)).to be_nil + end + + it "returns 0 h when given 0" do + expect(described_class.output(0)).to eq("0h") + end + + it "works with ChronicDuration defaults otherwise in :short format" do + expect(described_class.output(5.75)) + .to eq("5h 45m") + end + + it "handles floating point numbers gracefully" do + expect(described_class.output(0.28)) + .to eq("16m 48s") + end + end +end