Rails - May 2005 - Conditional validations

I''ve been working with the problem that Joe Hosteny has been talking
about on this list and on the tickets he''s posted
(http://dev.rubyonrails.com/ticket/1196) and have come up with a
tentative solution that I''d like to bring up here.

What if we use a "conditional" parameter in the validates_* methods? 
Here''s how it would look like in practice:

class Movie < ActiveRecord::Base
  def allowed_to_validate=(true_false)
    @allowed_to_validate = true_false
  end

  def allowed_to_validate
    @allowed_to_validate
  end
  
protected
  validates_length_of :title, :within => 1..30, :if =>
:allowed_to_validate
end

This would solve the problem that Joe discovered (one-way encrypted
attributes can''t be validated like other attributes) and it would also
give the programmer enough versatility to make validations only at key
junctures and only for certain attributes.  For example, I have a
4-part sign-up process that must record as much data as possible after
each step--it''s not good enough to keep it in the session and then
store it all in one save at the end.

The good news is that this modification to ActiveRecord::Validations
would be very simple.  In the validates_each method, I added the
following line:

          if options[:if] and record.send(options[:if])

... so that validates_each now looks like this:

      def validates_each(*attrs)
        options = attrs.last.is_a?(Hash) ? attrs.pop.symbolize_keys : {}
        attrs = attrs.flatten

        # Declare the validation.
        send(validation_method(options[:on] || :save)) do |record|
          if options[:if] and record.send(options[:if])
            attrs.each do |attr|
              value = record.send(attr)
              next if value.nil? && options[:allow_nil]
              yield record, attr, value
            end
          end
        end
      end

This modification covers the majority of cases; there were two other
validates_* methods that needed very minimal changes as well (which I
will omit here).

I''ve made patches before which were then given a "maybe" or
just plain
"no" so I''d like to know what the Powers That Be think of
this before
I submit a patch.

What does everyone think of this?

Regards,
-- 
Duane Johnson
(canadaduane)

Duane,

     That''s a much better solution than mine. In fact, I posted the  
patch hoping someone would suggest something like this, so thanks!  
The problem with mine is that it effectively gives you only one extra  
validation (probably enough, but why design in inflexibility?), where  
the conditional specifies when that particular validation applies.

     Just do what I did - post a patch, with unit tests that prove it  
works and is backward compatible, and an explanation of why you think  
it is justified, and wait to see if it gets picked up. I think this  
is useful and that it will be pulled in. I posted a lengthy  
explanation of why something like this is needed in 1196. If you''d  
like, feel free to commandeer that ticket and post your patch under  
it. I have some code that could be used to test it as well.

Joe

On May 4, 2005, at 5:28 PM, Duane Johnson wrote:
> I''ve been working with the problem that Joe Hosteny has been
talking
> about on this list and on the tickets he''s posted
> (http://dev.rubyonrails.com/ticket/1196) and have come up with a
> tentative solution that I''d like to bring up here.
>
> What if we use a "conditional" parameter in the validates_*
methods?
> Here''s how it would look like in practice:
>
> class Movie < ActiveRecord::Base
>   def allowed_to_validate=(true_false)
>     @allowed_to_validate = true_false
>   end
>
>   def allowed_to_validate
>     @allowed_to_validate
>   end
>
> protected
>   validates_length_of :title, :within => 1..30, :if  
> => :allowed_to_validate
> end
>
> This would solve the problem that Joe discovered (one-way encrypted
> attributes can''t be validated like other attributes) and it would
also
> give the programmer enough versatility to make validations only at key
> junctures and only for certain attributes.  For example, I have a
> 4-part sign-up process that must record as much data as possible after
> each step--it''s not good enough to keep it in the session and then
> store it all in one save at the end.
>
> The good news is that this modification to ActiveRecord::Validations
> would be very simple.  In the validates_each method, I added the
> following line:
>
>           if options[:if] and record.send(options[:if])
>
> ... so that validates_each now looks like this:
>
>       def validates_each(*attrs)
>         options = attrs.last.is_a?(Hash) ?  
> attrs.pop.symbolize_keys : {}
>         attrs = attrs.flatten
>
>         # Declare the validation.
>         send(validation_method(options[:on] || :save)) do |record|
>           if options[:if] and record.send(options[:if])
>             attrs.each do |attr|
>               value = record.send(attr)
>               next if value.nil? && options[:allow_nil]
>               yield record, attr, value
>             end
>           end
>         end
>       end
>
> This modification covers the majority of cases; there were two other
> validates_* methods that needed very minimal changes as well (which I
> will omit here).
>
> I''ve made patches before which were then given a "maybe"
or just plain
> "no" so I''d like to know what the Powers That Be think
of this before
> I submit a patch.
>
> What does everyone think of this?
>
> Regards,
> -- 
> Duane Johnson
> (canadaduane)
> _______________________________________________
> Rails mailing list
> Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org
> http://lists.rubyonrails.org/mailman/listinfo/rails
>