From 5c907c7f3fc626f3ca5a28258dd4b25ae8b8954c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrgen=20Tauschl?= Date: Fri, 14 Aug 2026 23:38:54 +0200 Subject: [PATCH 1/4] Fix 500 when creating working hours with a nil weekday The presence/numericality validation on each *_hours attribute invokes the corresponding dynamically-defined getter, which unconditionally divided the raw (possibly nil) minutes column by 60.0 -- a weekday omitted from a create payload has no DB default and stays nil, so validating it crashed with NoMethodError instead of producing a clean 422. A second, related nil-unsafe site exists in at_least_one_working_day_selected, reachable once every weekday is nil at once. Make both sites nil-safe: the getter returns nil for a nil column instead of dividing it, and the working-day check coerces via to_i first. --- app/models/user_working_hours.rb | 5 ++-- spec/models/user_working_hours_spec.rb | 37 ++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/app/models/user_working_hours.rb b/app/models/user_working_hours.rb index 013db8c2eff3..3e6c33c4bc76 100644 --- a/app/models/user_working_hours.rb +++ b/app/models/user_working_hours.rb @@ -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 define_method("#{day}_hours=") do |value| @@ -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 diff --git a/spec/models/user_working_hours_spec.rb b/spec/models/user_working_hours_spec.rb index e5b1c5a42c7e..ab205d38d4f8 100644 --- a/spec/models/user_working_hours_spec.rb +++ b/spec/models/user_working_hours_spec.rb @@ -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 @@ -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 From 5ff15ffe37f0b646d7669a1b8df63ff4b4aa3f9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrgen=20Tauschl?= Date: Sat, 15 Aug 2026 23:07:30 +0200 Subject: [PATCH 2/4] Guard DaysAndHoursForm#day_hours against a nil *_hours getter UserWorkingHours#{day}_hours now returns nil for a day with no minutes value at all (this branch's own earlier fix), but this form component called .round(2) on it unconditionally -- re-rendering the create form after a validation failure that left a weekday nil crashed with NoMethodError instead of showing the validation error banner. --- app/components/users/working_hours/days_and_hours_form.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/components/users/working_hours/days_and_hours_form.rb b/app/components/users/working_hours/days_and_hours_form.rb index 09b5d464d4b3..46115c6770d1 100644 --- a/app/components/users/working_hours/days_and_hours_form.rb +++ b/app/components/users/working_hours/days_and_hours_form.rb @@ -135,7 +135,9 @@ def day_enabled?(day) end def day_hours(day) - "#{model.public_send("#{day}_hours").round(2)}h" + # #{day}_hours is nil for a day with no minutes value at all (e.g. omitted from a create + # payload that failed validation) -- render it as 0h rather than crashing on nil.round. + "#{(model.public_send("#{day}_hours") || 0).round(2)}h" end def all_same_hours? From c0f9ac95cbb0ee024dfa58a46cce29cf8be3a079 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrgen=20Tauschl?= Date: Mon, 17 Aug 2026 16:09:24 +0200 Subject: [PATCH 3/4] Return 0.0 instead of nil for a day with no working-hours value A day with no minutes set has 0 working hours, not an undefined one -- callers like weekly_working_hours already summed day_hours without a nil guard. Also drops regression-test commentary that only restated what the test names and PR description already say. --- .../users/working_hours/days_and_hours_form.rb | 4 +--- app/models/user_working_hours.rb | 3 +-- spec/models/user_working_hours_spec.rb | 16 ++-------------- 3 files changed, 4 insertions(+), 19 deletions(-) diff --git a/app/components/users/working_hours/days_and_hours_form.rb b/app/components/users/working_hours/days_and_hours_form.rb index 46115c6770d1..09b5d464d4b3 100644 --- a/app/components/users/working_hours/days_and_hours_form.rb +++ b/app/components/users/working_hours/days_and_hours_form.rb @@ -135,9 +135,7 @@ def day_enabled?(day) end def day_hours(day) - # #{day}_hours is nil for a day with no minutes value at all (e.g. omitted from a create - # payload that failed validation) -- render it as 0h rather than crashing on nil.round. - "#{(model.public_send("#{day}_hours") || 0).round(2)}h" + "#{model.public_send("#{day}_hours").round(2)}h" end def all_same_hours? diff --git a/app/models/user_working_hours.rb b/app/models/user_working_hours.rb index 3e6c33c4bc76..af859178985c 100644 --- a/app/models/user_working_hours.rb +++ b/app/models/user_working_hours.rb @@ -72,8 +72,7 @@ def self.current DAYS.each do |day| define_method("#{day}_hours") do - minutes = public_send(day) - minutes.nil? ? nil : (minutes / 60.0).round(2) + ((public_send(day) || 0) / 60.0).round(2) end define_method("#{day}_hours=") do |value| diff --git a/spec/models/user_working_hours_spec.rb b/spec/models/user_working_hours_spec.rb index ab205d38d4f8..55bb86720233 100644 --- a/spec/models/user_working_hours_spec.rb +++ b/spec/models/user_working_hours_spec.rb @@ -76,10 +76,6 @@ 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) @@ -89,10 +85,6 @@ 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 @@ -114,13 +106,9 @@ 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. + it "returns 0.0 rather than raising when the underlying minutes column is nil" do working_hours.public_send("#{day}=", nil) - expect(working_hours.public_send("#{day}_hours")).to be_nil + expect(working_hours.public_send("#{day}_hours")).to eq(0.0) end end From 1c1a98e18658bcf0ff48e42a8967841919e54862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrgen=20Tauschl?= Date: Tue, 18 Aug 2026 08:49:08 +0200 Subject: [PATCH 4/4] Validate raw weekday minutes instead of the *_hours getter validates :monday_hours, ... presence:/numericality: silently stopped catching a missing weekday once the getter started returning 0.0 instead of nil (0.0 satisfies both checks). Validate the underlying minutes column directly -- nil only when the day is genuinely missing -- and keep reporting the error under the public *_hours name. --- app/models/user_working_hours.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/app/models/user_working_hours.rb b/app/models/user_working_hours.rb index af859178985c..922d0c132600 100644 --- a/app/models/user_working_hours.rb +++ b/app/models/user_working_hours.rb @@ -38,14 +38,12 @@ class UserWorkingHours < ApplicationRecord belongs_to :user, inverse_of: :working_hours validates :valid_from, presence: true, uniqueness: { scope: :user_id } - validates :monday_hours, :tuesday_hours, :wednesday_hours, :thursday_hours, :friday_hours, :saturday_hours, :sunday_hours, - presence: true, - numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: 24 } validates :availability_factor, presence: true, numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 100 } validate :at_least_one_working_day_selected + validate :working_day_hours_present_and_in_range scope :for_user, ->(user) { where(user:) } @@ -162,4 +160,17 @@ def at_least_one_working_day_selected errors.add(:days, :no_working_day) end end + + def working_day_hours_present_and_in_range + DAYS.each do |day| + minutes = public_send(day) + attr = :"#{day}_hours" + + if minutes.nil? + errors.add(attr, :blank) + elsif minutes.negative? || minutes > 24 * 60 + errors.add(attr, :less_than_or_equal_to, count: 24) + end + end + end end