From 178e8cad250696d0ade352a119b5f78c3f47227b Mon Sep 17 00:00:00 2001
From: Klaus Zanders <k.zanders@openproject.com>
Date: Fri, 6 Dec 2024 15:46:32 +0100
Subject: [PATCH] Add conversion from HH:mm format to integer

---
 modules/costs/app/models/time_entry.rb       | 17 +++++++++--
 modules/costs/config/locales/en.yml          |  2 ++
 modules/costs/spec/models/time_entry_spec.rb | 32 +++++++++++++++++---
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/modules/costs/app/models/time_entry.rb b/modules/costs/app/models/time_entry.rb
index 7ca7f424c695..32048cb152c8 100644
--- a/modules/costs/app/models/time_entry.rb
+++ b/modules/costs/app/models/time_entry.rb
@@ -52,7 +52,12 @@ class TimeEntry < ApplicationRecord
             if: -> { TimeEntry.must_track_start_and_end_time? }
 
   validates :start_time,
-            numericality: { only_integer: true, greater_than_or_equal_to: MIN_TIME, less_than_or_equal_to: MAX_TIME },
+            numericality: {
+              only_integer: true,
+              greater_than_or_equal_to: MIN_TIME,
+              less_than_or_equal_to: MAX_TIME,
+              message: :invalid_time
+            },
             allow_blank: true
 
   scope :on_work_packages, ->(work_packages) { where(work_package_id: work_packages) }
@@ -71,7 +76,7 @@ class TimeEntry < ApplicationRecord
   register_journal_formatted_fields "hours", formatter_key: :time_entry_hours
   register_journal_formatted_fields "user_id", formatter_key: :time_entry_named_association
   register_journal_formatted_fields "work_package_id", "activity_id", formatter_key: :named_association
-  register_journal_formatted_fields "comments", "spent_on", formatter_key: :plaintext
+  register_journal_formatted_fields "comments", "spent_on", "start_time", formatter_key: :plaintext
 
   def self.update_all(updates, conditions = nil, options = {})
     # instead of a update_all, perform an individual update during work_package#move
@@ -92,6 +97,14 @@ def hours=(value)
     write_attribute :hours, (value.is_a?(String) ? (value.to_hours || value) : value)
   end
 
+  def start_time=(value)
+    if value.is_a?(String) && value =~ /\A(\d{1,2}):(\d{2})\z/
+      super(($1.to_i * 60) + $2.to_i)
+    else
+      super
+    end
+  end
+
   # Returns true if the time entry can be edited by usr, otherwise false
   def editable_by?(usr)
     (usr == user && usr.allowed_in_work_package?(:edit_own_time_entries, work_package)) ||
diff --git a/modules/costs/config/locales/en.yml b/modules/costs/config/locales/en.yml
index f5c2ca30d0b5..39542b6d1af9 100644
--- a/modules/costs/config/locales/en.yml
+++ b/modules/costs/config/locales/en.yml
@@ -71,6 +71,8 @@ en:
       rate: "Rate"
     errors:
       models:
+        time_entry:
+          invalid_time: "must be between 00:00 and 23:59."
         work_package:
           is_not_a_valid_target_for_cost_entries: "Work package #%{id} is not a valid target for reassigning the cost entries."
           nullify_is_not_valid_for_cost_entries: "Cost entries can not be assigned to a project."
diff --git a/modules/costs/spec/models/time_entry_spec.rb b/modules/costs/spec/models/time_entry_spec.rb
index 8f3fa58884d8..20389f5cf12b 100644
--- a/modules/costs/spec/models/time_entry_spec.rb
+++ b/modules/costs/spec/models/time_entry_spec.rb
@@ -84,7 +84,7 @@ def ensure_membership(project, user, permissions)
            roles: [create(:project_role, permissions:)])
   end
 
-  describe "#hours" do
+  describe "#hours=" do
     formats = { "2" => 2.0,
                 "21.1" => 21.1,
                 "2,1" => 2.1,
@@ -104,13 +104,28 @@ def ensure_membership(project, user, permissions)
 
     formats.each do |from, to|
       it "formats '#{from}'" do
-        t = TimeEntry.new(hours: from)
+        t = described_class.new(hours: from)
         expect(t.hours)
           .to eql to
       end
     end
   end
 
+  describe "#start_time=" do
+    formats = {
+      "720" => 720,
+      "12:00" => 720,
+      "13:37" => 817
+    }
+
+    formats.each do |from, to|
+      it "formats '#{from}'" do
+        t = described_class.new(start_time: from)
+        expect(t.start_time).to eql(to)
+      end
+    end
+  end
+
   it "always prefers overridden_costs" do
     allow(User).to receive(:current).and_return(user)
 
@@ -424,6 +439,17 @@ def ensure_membership(project, user, permissions)
         expect(time_entry).to be_valid
       end
 
+      it "allows string time values" do
+        time_entry.start_time = "12:00"
+        expect(time_entry).to be_valid
+      end
+
+      it "does not allow times > 23:59" do
+        time_entry.start_time = "26:00"
+        expect(time_entry).not_to be_valid
+        expect(time_entry.errors.full_messages).to include("Start time must be between 00:00 and 23:59.")
+      end
+
       it "does not allow non integer values" do
         time_entry.start_time = 1.5
         expect(time_entry).not_to be_valid
@@ -433,9 +459,7 @@ def ensure_membership(project, user, permissions)
         time_entry.start_time = -42
         expect(time_entry).not_to be_valid
       end
-    end
 
-    describe "start_time and end_time" do
       context "when enforcing times" do
         before do
           allow(described_class).to receive(:must_track_start_and_end_time?).and_return(true)