Re: [Rails-core] custom validations only if all attributes are valid

[Steve Klabnik]

I think I'd be much more annoyed by this kind of behavior. "Okay,
these two things are wrong. Let's fix them. Wait, now it's _still_
wrong, with something totally unrelated?"

You could fix this case almost trivially by 'if age.to_i < 16', right?
I mean, I know you're talking about a more general case, but...

[Carlos Antonio da Silva]

I agree with Steve, when we call valid?, we expect all validations to run. If you want to skip some validation in case errors already exist in the object for any particular reason, you can always to that as you said: `if errors.empty?`. This will give you the behavior you expect, although I'd recommend avoiding this and running all validations.

-- 

At.

Carlos Antonio

--

You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.

To post to this group, send email to rubyonra...@googlegroups.com.

To unsubscribe from this group, send email to rubyonrails-co...@googlegroups.com.

For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.

[Ken Collins]

Agreed, and by "running all validations" you can have a certain validator that acts as a state machine on a set of validations that gives more meaningful errors as the state moves forward. I think most of us have done that in the past to get the behavior you are looking for. Changing core behavior for that case does not make sense.

 - Ken

[Anthony Richardson]

I can see some advantge of declaring a validation dependency. Checking
if an age is less than 16 makes no sense if the age has failed a
validation on being a number already.

maybe something like:

class User < AR::Base
  validates_numericality_of :age

  #custom validation
  validate :user_older_16, :depends => :age

  def user_older_16
    errors.add(:age, "should be older 16") if age <= 16
  end
end

Where :user_older_16 would only run if age has no errors. In the
context shown above this is a sensible dependency, but used
incorrectly the concerns expressed by others would be valid.

Another suggestion:

class User < AR::Base

validates_with_dependency do
  validates_numericality_of :age
validate :user_older_16
end

  def user_older_16
    errors.add(:age, "should be older 16") if age <= 16
  end
end

In the above the validations in the validate_with_dependency block
would execute in order, exiting the sequence if any fail.

I think the suggestion to have dependent validaiton is a good one as
it allows for structuring simiple validations where you know a
validation cannot possibly pass if an earlier one has failed.

Cheers,

Anthony Richardson.

On Thu, Jun 21, 2012 at 8:26 PM, Pavel Gabriel <alo...@gmail.com> wrote:
> Hi!
>
> I found that it's a popular use case for AR/AM validations:
>
> class User < AR::Base
>   validates_numericality_of :age
>
>   #custom validation
>   validate :user_older_16
>
>   def user_older_16
>     errors.add(:age, "should be older 16") if age <= 16
>   end
> end
>
> When you will try to validate as the following:
>
>   user =  User.new(age: 'bla')
>   user.valid?
>
> you will get: ArgumentError: comparison of String with 16 failed.
>
> Folks write a lot of code to check attribute values in their custom
> validations, but it's not a DRY way because attributes already validated by
> AM.
>
> Is it a good idea to run validation methods only if errors.count == 0?
>
> Or maybe we can add something like "validate_further :user_older_16" that
> will run only if attributes are valid?

>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.

> To view this discussion on the web visit
> https://groups.google.com/d/msg/rubyonrails-core/-/Ca3mpVVPWlgJ.

[Rodrigo Rosenfeld Rosas]

Yeah, I couldn't understand what Ken meant by his "state-machine" approach.

I can't think of a dry way of currently doing that.

I'm just not much concerned about this as I don't use AR and Sequel's
way of doing validation is by defining a "validate" method, so I'd put
my logic there, but anyway...

[Ken Collins]

A poor choice of words maybe, I was trying to say that at some point in time, I have had models that needed a hierarchy of potential errors and that some errors are not appropriate to show at certain times. A object that goes thru states is a good example.

- Ken

[Pavel Gabriel]

Steve, It's easy to reply to your post, because you have a code example :)

In you case, you will have to duplicate all checks in your custom validations, that is not DRY approach.

You will duplicate .to_i in all methods that will depends on age as integer.

[Pavel Gabriel]

Carlos,

I agree with you. Technically I can check that errors are empty, but I have to do it in all my custom validations.

it will not look nice :(

What I offer is a two stage validation. When on the second stage you will be sure that all data in correct state and you can use it with confidence.

Anyway you will rely on one valid? method.

What I added into my project is:

https://gist.github.com/2987785

The reason I did it is a lot of exceptions we got when our clients didn't provide all necessary data (or in correct format).
The obvious solution of course was to manually check all data in custom validations, but why I need to duplicate code if AR/AM already implemented it?

On Thursday, June 21, 2012 4:07:09 PM UTC+3, Carlos Antonio da Silva wrote:

I agree with Steve, when we call valid?, we expect all validations to run. If you want to skip some validation in case errors already exist in the object for any particular reason, you can always to that as you said: `if errors.empty?`. This will give you the behavior you expect, although I'd recommend avoiding this and running all validations.

-- 

At.

Carlos Antonio

On Thursday, June 21, 2012 at 9:51 AM, Steve Klabnik wrote:

I think I'd be much more annoyed by this kind of behavior. "Okay,

these two things are wrong. Let's fix them. Wait, now it's _still_

wrong, with something totally unrelated?"

You could fix this case almost trivially by 'if age.to_i < 16', right?

I mean, I know you're talking about a more general case, but...

--

You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.

To post to this group, send email to rubyonrails-core@googlegroups.com.

To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com.

[Pavel Gabriel]

Anthony, good point!

validates_with_dependency is the a specific case of suggested 'validate_further' approach.

Two stage validation doesn't require you to manage dependencies (order, etc.) so no need to specify them.

You just rely on results from first stage.

One more example from real project:

order.rb

validates_presence_of :shop
validates_presence_of :line_items
validates_associated :billing_address

...
 
validate_further :on => :create do
  errors.add(:signature, :invalid) unless signature_verified?
end

method signature_verified? calculates signature with

• shop.secret_key  - shop should present
  
• amount, quantity, name for each line_item
• attributes for billing/shipping, etc.

and compares it with received value from user

do I really have to check manually that everything valid before calculate signature?

> To post to this group, send email to rubyonrails-core@googlegroups.com.

> To unsubscribe from this group, send email to

> rubyonrails-core+unsubscribe@googlegroups.com.

[Andrew White]

If you only want to run a validation on an attribute if it is valid at that point in the validation process then here's a trick:

module AttributeValidation
extend ActiveSupport::Concern

included do
attribute_method_suffix '_valid?', '_invalid?'
end

private

def attribute_valid?(attr)
!attribute_invalid?(attr)
end

def attribute_invalid?(attr)
errors.include?(attr.to_sym)
end
end

include this module in AR::Base or specific models - it's up to you. Once included you can then set up your validations like this:

class User < ActiveRecord::Base
include AttributeValidation

attr_accessible :name, :age

validates :name, presence: true, length: { maximum: 100 }
validates :age, presence: true, numericality: { only_integer: true }
validate :over_minimum_age, if: :age_valid?

def over_minimum_age
errors.add(:age, :invalid) if age < 16
end
end

this will only run the minimum age validation once it has passed the presence and numericality checks. One advantage of doing this is you don't need guard checks when comparing the age (normally you'd get a "NoMethodError: undefined method `<' for nil:NilClass")


Andrew

[Pavel Gabriel]

Guys,

there are people who see a value in validation dependency and who don't.

I want to add this to Rails core and ready to make a pull request.
But I need some help about the name of the method (validate_after, validate_with_dependency, validate_further, etc.)   and usage of this approach.

I posted earlier what I use on my project. I think that the interface of this method should be the same with validate method (:on, :if/:unless).