Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions app/models/user_working_hours.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ def self.current

DAYS.each do |day|
define_method("#{day}_hours") do
(public_send(day) / 60.0).round(2)
minutes = public_send(day)
minutes.nil? ? nil : (minutes / 60.0).round(2)
end
Comment on lines 74 to 77

define_method("#{day}_hours=") do |value|
Expand Down Expand Up @@ -158,7 +159,7 @@ def abbr_day_name(day)
end

def at_least_one_working_day_selected
if DAYS.all? { |day| public_send(day).zero? }
if DAYS.all? { |day| public_send(day).to_i.zero? }
errors.add(:days, :no_working_day)
end
end
Expand Down
37 changes: 37 additions & 0 deletions spec/models/user_working_hours_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,34 @@
.is_greater_than_or_equal_to(0)
.is_less_than_or_equal_to(100)
end

describe "with a nil weekday column (e.g. omitted from a create payload)" do
# Regression tests: presence/numericality validation on a `*_hours`
# attribute invokes the corresponding getter, which used to crash with
# NoMethodError on `nil / 60.0` instead of producing a clean
# validation error.
it "is invalid rather than raising when a single weekday is nil" do
subject.public_send(:wednesday=, nil)

expect { subject.valid? }.not_to raise_error
expect(subject).not_to be_valid
expect(subject.errors[:wednesday_hours]).to be_present
end

it "is invalid rather than raising when every weekday is nil" do
# Also exercises the second, independent nil-unsafe site in
# at_least_one_working_day_selected, only reachable once every
# weekday is nil (each weekday's own presence/numericality
# validation would otherwise short-circuit first).
%i[monday tuesday wednesday thursday friday saturday sunday].each do |day|
subject.public_send(:"#{day}=", nil)
end

expect { subject.valid? }.not_to raise_error
expect(subject).not_to be_valid
expect(subject.errors[:days]).to be_present
end
end
end

describe "hours accessors" do
Expand All @@ -85,6 +113,15 @@
working_hours.public_send("#{day}=", 150)
expect(working_hours.public_send("#{day}_hours")).to eq(2.5)
end

it "returns nil rather than raising when the underlying minutes column is nil" do
# Regression test: a still-nil column (e.g. a weekday omitted from
# a create payload, no DB default) used to crash with
# NoMethodError on `nil / 60.0` -- both here, and via the
# presence/numericality validator that calls this same getter.
working_hours.public_send("#{day}=", nil)
expect(working_hours.public_send("#{day}_hours")).to be_nil
end
end

describe "##{day}_hours=" do
Expand Down
Loading