Inconsistant behaviour when validating on a method and not a column

[Aquaj]

I stumbled upon a weird behaviour when trying to use allow_blank.

We have an Intervention model, with a legacy started_at column. It also has a started_at method (that looks into dependent working_periods to compute it).

In one instance, the column is filled but the methods returns nil. The validator doesn't skip the validation since the column is filled but the validation breaks since the method returns nil.

I think a more coherent behaviour would be to only take into account the method's return value — which is what ActiveModel default validators do.

I think the fix should be around in #validate_each, maybe adjusting the guard-clause or the value setting before the blankness-check.

How to reproduce

class Intervention < ActiveRecord::Base
  validates :started_at, timeliness: { on_or_before: -> { Time.now } }, allow_blank: true

  def started_at
    nil
  end
end

Intervention.new(started_at: Time.now - 1.hour).tap(&:valid?).errors.messages
# => { :started_at => ["Started at is not a valid datetime"] } 

[adzap]

I think the best comparison to make is with validates_numericality_of where if you have an integer column and assign a junk string, the error of not a number is returned even if you have allow_blank: true. That is the behaviour that is being matched here.

[adzap]

Though, admittedly in the case of numericality it casts junk as 0 (grrr), so the column doesn't return nil.

Basically I consider blank, when there has been no attempt to cast a value to a datetime. But I guess that could be what just allow_nil is for.

I certainly don't want to cast to some dummy value like the number case I outlined. So this interpretation might be in a category all its own.

[adzap]

In your case, what value is the column being filled with to trigger the validation?

[Aquaj]

The column isn't actually filled with junk but with a proper value (a DateTime in this case).

To take the example of validates_numericality and put it in contrast with validates_timeliness:

class Intervention < ActiveRecord::Base
  validates :duration, numericality: { greater_than: 0, allow_blank: true }
  validates :started_at, timeliness: { on_or_before: -> { Time.now }, allow_blank: true }

  def duration
    nil
  end

  def started_at
    nil
  end
end

Intervention.new(duration: -3).valid? # => true. It detects the #duration method value as nil and skips the validation
# but
Intervention.new(started_at: Time.now - 1.hour).valid? # => false ("Invalid datetime"). Detects attributes[:started_at] as not-nil so doesn't skip, but uses the #started_at method value (nil) as the date to validate.

[joaomangilli]

The problem happen because the raw_value looks to attribute value and don't to getter method: https://github.com/adzap/validates_timeliness/blob/master/lib/validates_timeliness/validator.rb#L99.

Maybe it would be the case to check the value, some like that: return if (@allow_nil && raw_value.nil? || value.nil?) || (@allow_blank && raw_value.blank? || value.blank?)