Extract parsing and formatting logic for duration into service

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]

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",

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

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