Skip to content

Commit

Permalink
Add conversion from HH:mm format to integer
Browse files Browse the repository at this point in the history
  • Loading branch information
klaustopher committed Dec 10, 2024
1 parent 9683cd1 commit 178e8ca
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 6 deletions.
17 changes: 15 additions & 2 deletions modules/costs/app/models/time_entry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand All @@ -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
Expand All @@ -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)) ||
Expand Down
2 changes: 2 additions & 0 deletions modules/costs/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
32 changes: 28 additions & 4 deletions modules/costs/spec/models/time_entry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)

Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down

0 comments on commit 178e8ca

Please sign in to comment.