Possible bug in Validations

Hello all,

I think I have found an interesting bug in the validations. The
reason I say might is because I don't know if it was intentionally
constructed this way or not.

ActiveRecord docs describe the validates_as_* methods as accepting
the :on argument with the options of :create, :update and :save
(default :save). If you explicitly set argument to :on => :save that
validation if forever skipped. The context or state of the model
(:create or :update) does not matter.

class Topic < ActiveRecord::Base
  validates_presence_of :title, :on => :save
end

t = Topic.new(:title => '')
t.valid? => true

t = Topic.find(1)
t.title = ''
t.valid? => true

I have verified this with tests in ActiveRecord.

My question is should this be fixed so that :save is a valid option or
should it be left as is and the documentation updated to no longer
show :save as a valid option?

Related to this, if you attempt to send the :on argument to a
validates_* of a class that uses ActiveModel::Validations#valid?
(ActiveRecord overrides it) to do its validation then they will not
work.

class Foo
  include ActiveModel::Validations

  attr_accessor :title

  validates_presence_of :title, :on => :save
end

f = Foo.new
f.title = ''
f.valid? => true

Again, is this expected behaiour?

Peer

I realized right after I hit "Send" that the second half, the
ActiveModel related part, of my post was completely irrelevant. Of
course ActiveModel would fail that way if you don't supply the
context! Oops.

Peer

Docs are wrong, :save is not the default, the default is that the context will be set (to either :create or :update) based on new_record? - :save is never a valid value for :on unless you’re going to pass that context to your valid? and save calls, eg. this will both work correctly with :on => :save in your validation(s):

valid? :save # => false

save :context => :save # => false

But obviously that’s not really what the docs were trying to suggest :slight_smile:

Agreed, the documentation is wrong. The question now is should the
documentation just be updated or should some kind of deprecation
warning or exception be put in place to identify this change to users
who are upgrading their 2.3.x applications? I found this while
upgrading our 2.3.x application. A validation test was failing, but
there was no indication as to why. It took a while to figure out the
:on value was to blame.

Peer

:on => :save worked in 2.3? Sounds like something pretty simple to add to rails_upgrade.

Of course, that is the place to put it (and the easiest!).

I've patched that plugin before and submitted it via lighthouse, but
that was several months ago and I've haven't received any feedback on
it. Is someone actively maintaining it through rails or should I look
to get it pushed through github?

Peer

Next question, most of the documentation related to this is in
ActiveModel which states,

  # * <tt>:on</tt> - Specifies when this validation is active (default
is <tt>:save</tt>, other options <tt>:create</tt>, <tt>:update</tt>).

This description is inaccurate for ActiveModel as it can receive
anything as a context, the :create and :update options are valid when
using ActiveRecord. Would updating the documentation in ActiveModel to
remove the valid options cause confusion? For example,

  # * <tt>:on</tt> - Specifies the context when this validation is
active (e.g. <tt>:on => :create</tt>, or <tt>:on => :special_rules</

)

This is true for the way ActiveModel works, but not helpful when using
ActiveRecord. Any suggestions on how to make it less confusing?

Peer

After submitting patches, drop a line here to get some interest and +1 votes for your patch.