Possible bug in Validations

rubyonrails-core
1

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

2

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

3

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:

4

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

5

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

6

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

7

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

8

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