Rails validation method comparing two fields?

My model has two fields that i want to compare to each other as part of the validation. I want to be sure end_time is after start_time. I have written a validation method to compare them, but I must be doing something wrong, because the values are always nil. Can someone help?

class LogEntry < ActiveRecord::Base
  validates :start_time, :presence => { :message => "must be a valid date/time" }
  validates :end_time, :presence => {:message => "must be a valid date/time"}
  validate :start_must_be_before_end_time

  def start_must_be_before_end_time
    errors.add(:start_time, "must be before end time") unless
       start_time > end_time
  end 
end

gets the error

undefined method `>' for nil:NilClass

So, start_time and/or end_time are nil. I thought I was following the many examples I found, but apparently not. What am I missing?

Thanks.

Solution 1:

My best guess is you need your method to look like this:

private

def start_must_be_before_end_time
    errors.add(:start_time, "must be before end time") unless
        start_time < end_time
end 

(Also, notice the < rather than > (or change to if and >=)

If this doesn't work then you should also check that start_time and end_time are being defined correctly in the controller as there can be funny things happen if the time is created across more than one form element.

With Rails 7 ComparisonValidator

Rails 7 has added the ComparisonValidator which allows you to use the convenience method validates_comparison_of like so:

class LogEntry < ActiveRecord::Base
  validates_comparison_of :start_time, less_than: :end_time
  # OR
  validates_comparison_of :end_time, greater_than: :start_time
end

Find out more here: https://api.rubyonrails.org/classes/ActiveModel/Validations/HelperMethods.html#method-i-validates_comparison_of

Solution 2:

You need to check for presence yourself (and skip the validation step if not present).

def start_must_be_before_end_time
  return unless start_time and end_time
  errors.add(:start_time, "must be before end time") unless start_time < end_time
end

Prints either "must be a valid date/time" OR "start time must be before end time".

Alternative

def start_must_be_before_end_time
  valid = start_time && end_time && start_time < end_time
  errors.add(:start_time, "must be before end time") unless valid
end

Prints "start time must be a valid date/time" AND "start time must be before end time" if start_time or end_time isn't set.

Personal preference for the first, since it only shows what the user did wrong. The latter is like many websites which just load off 20 lines of error text onto the user just because the programmer thought it would be nice to see every validation result. Bad UX.

Solution 3:

Clean and Clear (and under control?)

I find this to be the clearest to read:

In Your Model

# ...

validates_presence_of :start_time, :end_time

validate :end_time_is_after_start_time

# ... 

#######
private
#######

def end_time_is_after_start_time
  return if end_time.blank? || start_time.blank?

  if end_time < start_time
    errors.add(:end_time, "cannot be before the start time") 
  end 
end