Datamapper DataMapper → 0.10.0

Comments and changes to this ticket

• 

  Dan Kubb (dkubb) January 8th, 2009 @ 02:44 PM

  • Assigned user set to “Michael Klishin (antares)”
• 

  Michael Klishin (antares) March 25th, 2009 @ 04:59 PM

  • Milestone set to “0.10.0”
  • State changed from “unconfirmed” to “confirmed”
• 

  Michael Klishin (antares) March 28th, 2009 @ 09:55 AM

  • Tag set to “dm-validations, validations”
  • State changed from “confirmed” to “accepted”
• 

  Michael Klishin (antares) March 31st, 2009 @ 01:40 PM

  I could reproduce it. On it now.

• 

  Michael Klishin (antares) March 31st, 2009 @ 01:50 PM

  • State changed from “accepted” to “not-applicable”

  After some investigation I figured out that it is a doc error that leads to misunderstanding. First argument in the case provided by OP is not a property name, but yet another method that is being called in the instance. And that method also required to follow validating method rules and either return true, or a pair of [false, error message].

  Here is what it looks in the code of method validator:

  
        def initialize(field_name, options={})
          super
          @options[:method] = @field_name unless @options.has_key?(:method)
        end
  
        def call(target)
          result, message = target.send(@options[:method])
          add_error(target, message, field_name) unless result
          result
        end
  

  So, since some_property on a newly initialized instance is nil, and method :some_property returns it, message (that is also nil) is added to object errors. I am going to correct the docs now.

• 

  Michael Klishin (antares) March 31st, 2009 @ 01:55 PM

  • State changed from “not-applicable” to “confirmed”

  One more thing. It may be a desired behavior for 1st argument to be considered property name. However, I found that method and block validators are often used for virtual attributes.

  I am not sure what the behavior should be in this case. You easily can mimic behavior where 1st argument is a property name with a block validator. On the other hand I realize that a lot of people would expect the same thing as OP.

• 

  Michael Klishin (antares) March 31st, 2009 @ 02:27 PM

  • State changed from “confirmed” to “not-applicable”

  After discussion in developers channel on the IRC I have to make this hard but necessary decision: current behavior is here to stay with documentation corrections that explains my reasoning:

  
  antares_: namelessjon: see, all other validators take arbitrary number of field names and options hash
  antares_: here we would go against that, with 1st argument being special, and the rest being method names
  antares_: :method => :method_name that works right now as dbussink pointed out, does not struggle from this inconsistency
  antares_: validates_with_method :prop1, :prop2, :prop3, :method => :validator will validate 3 properties with one method
  antares_: probably makes little sense, but is 100% consistent with other validators
  antares_: it is also not clear how often people validate many properties with method validator
  antares_: method validator is less commonly used from my experience
  antares_: another reason is to not change validator behavior
  antares_: validator behavior changed may mean data inconsistency in user apps
  antares_: and it is just.. a too serious problem
  antares_: a few invalid objects slip through and you may have lost control over situation
  antares_: so I'll probably document current behavior better, and suggest to only use validates_method with one method at a time
  

  Let me know if you totally disagree :)

• You flagged this item as spam.
  

  Jeremy Nicoll March 31st, 2009 @ 05:18 PM

  Makes sense, actually. I'm someone who makes fairly liberal use of custom validations. I'm someone who sometimes likes to add multiple error messages with one method. I'd be just fine manually adding errors to the error field and then returning false from the method. I'd prefer being able to define multiple methods with one call to validates_with_method as well.