Rails Ruby on Rails

Comments and changes to this ticket

• You flagged this item as spam.
  

  Josh Susser May 4th, 2008 @ 08:01 PM

  I think I'd find this useful, though please don't build your API around a meta send, it's not really unnecessary and adds overhead. It's just as easy to implement in a way that's backward compatible and without the send.

  def errors(level=nil) # in AR::Base
  
  record.errors.add_to_base("Something is wrong")
  record.errors(:warnings).add_to_base("Something is wrong")
  

  Or something like that. A more generic name for #errors like #validation_failures might make more sense, and make #errors call validation_failures with for backward compatibility.

  I have some code that could use this feature to clean up a multilevel validation scheme, though it's not edge-compatible yet. I might try to play around with this approach anyway. If I've got so many comments on your code I should probably answer it with a patch.

• 

  Michael Koziarski May 4th, 2008 @ 11:12 PM

  • Assigned user set to “Michael Koziarski”

  errors(nil) certainly seems like a reasonable approach, do you have any thoughts michael?

• You flagged this item as spam.
  

  Adam Meehan May 4th, 2008 @ 11:43 PM

  I think I would find this useful also.

  To DRY it up a little could you not put the level extraction into the validates_each block above the loop so you only need to call it in one place. i.e.

  {{{

  def validates_each(*attrs)

  options = attrs.extract_options!.symbolize_keys

  attrs = attrs.flatten

  level = validation_level(options)

  1. Declare the validation.

  send(validation_method(options[:on] || :save), options) do |record|

  attrs.each do |attr|

  value = record.send(attr)

  next if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank])

  yield record, attr, value

  end

  end

  end

  }}}

  If it was tied into save so that as well as false you could pass a symbol or hash so you could be specific as to which error level was to be ignored on save. e.g

  {{{

  save(false, :allow => :warnings)

  }}}

• You flagged this item as spam.
  

  Adam Meehan May 4th, 2008 @ 11:46 PM

  Sorry wrong formatting.

  I think I would find this useful also.

  To DRY it up a little could you not put the level extraction into the validates_each block above the loop so you only need to call it in one place. i.e.

  def validates_each(*attrs)
    options = attrs.extract_options!.symbolize_keys
    attrs   = attrs.flatten
  
    level = validation_level(options)
  
    # Declare the validation.
    send(validation_method(options[:on] || :save), options) do |record|
      attrs.each do |attr|
        value = record.send(attr)
        next if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank])
        yield record, attr, value
      end
    end
  end
  

  If it was tied into save so that as well as false you could pass a symbol or hash so you could be specific as to which error level was to be ignored on save. e.g

  save(false, :allow => :warnings)
  

• You flagged this item as spam.
  

  Michael Schuerig May 5th, 2008 @ 01:01 AM

  Josh, Koz: I'm fine with

  def errors(level = nil)
    level ||= :errors
    ...
  

  Adam: I understand your DRY intention, but not how it's going to work. level isn't even used inside the block.

  save with an option to allow validation failures up to a certain level sounds interesting and is something I didn't think of. There are two aspects that make me hesitate:

  1. The first boolean arg and the latter :allow option are not orthogonal. I think the first arg should be either true/false or a hash.
  2. The functionality could be added by a plugin and doesn't have to go into core.

• You flagged this item as spam.
  

  Adam Meehan May 5th, 2008 @ 01:56 AM

  Ok, I didn't see any use of the level variable outside of the validates_each block in your patch, with the exception of 'validates_presence_of'.

  My example is incomplete as I left out level in the yield. But on seconds thoughts, it would break existing plugins. So perhaps not.

  1. Agreed, was just first thought. A hash or boolean might be clearer.

  2. It would be useful in some of my work, so a plugin might be on the cards.

• You flagged this item as spam.
  

  Adam Meehan May 5th, 2008 @ 02:19 AM

  Actually a cleaner solution than my first is to just add

  :level => :errors

  in the default configuration hash for each validation method. No need for an extra method to extract and pluralize it. Then just use configuration[:level] where needed.

• You flagged this item as spam.
  

  Michael Schuerig May 5th, 2008 @ 07:18 AM

  Using :error instead of :errors to designate the level was deliberate. To me :level => :error, :level => :warning, sound more natural and more in line with logging levels. However, I'm easily convinced otherwise.

• You flagged this item as spam.
  

  Adam Meehan May 5th, 2008 @ 08:26 AM

  Your right it does sound more natural. Perhaps just leave the level singular and when a plural symbol is passed to the errors method as per Josh's suggestion then you can singularize it.

  Anyway enough of my musings. Lets see what the other guys think.

• 

  Pratik May 14th, 2008 @ 12:17 PM

  I'm very tempted to say "Plugin material".

• You flagged this item as spam.
  

  Michael Schuerig May 14th, 2008 @ 12:34 PM

  Pratik, unfortunately this can't be done in a plugin without copying almost all of the validations -- which would be subject to break with changes in Rails.

  The idea is precisely to add these hooks to make them usable by plugins. The proposed changes, in either version, are backward compatible and robust in the face of changes.

• 

  josh May 14th, 2008 @ 05:20 PM

  Errr, I really don't like this either. How could we refactor validations so you could write a plugin easier?

• You flagged this item as spam.
  

  Michael Schuerig May 14th, 2008 @ 05:58 PM

  I have no better idea at the moment. Put abstractly, there needs to be a way to parameterize where a failed validation add its message.

• 

  Pratik May 20th, 2008 @ 01:31 PM

  Removing "patch" keyword.

  Also, I think it's worth noting that very soon we'll be moving validation stuff to ActiveModel, which already has some API changes. Probably, you should try to email core list about this feature request and find out what everyone else thinks.

  Thanks.

• You flagged this item as spam.
  

  Michael Schuerig May 20th, 2008 @ 01:45 PM

  I already send a message to rails-core on 2008-05-03, even before writing the patch. Koz advised me to write a patch and so I did.

  Regarding ActiveModel: Yes, that would be the right place to put this, however, I have no idea what the current state of ActiveModel is or if anyone is working on it.

• 

  josh August 20th, 2008 @ 05:16 PM

  • State changed from “new” to “stale”
  • Tag set to “activerecord, enhancement”