Association validation

[xaviershay]

Currently, there is a confusing behaviour with validations:

require 'dm-validations'

class User
  include DataMapper::Resource

  has n, :posts
end

class Post
  belongs_to :user

  property :id, Serial
  property :title, String, :required => true
end

user = User.new(:posts => [{:title => ''}])
user.valid? # => true
user.save   # => false

This in particular crops up as an issue when using dm-nested-attributes. These commits add auto-validation to associations to make #valid? return false in the above example. There are also safe-guards to prevent "known good" children from being validated, without which saving a parent model could trigger a massive chain of child validations. "Known good" is defined as either not loaded or clean.

There is still a pending issue that has 1 associations are always loaded, but validations are not run on them if they are clean (which they will be if they were just loaded). This isn't a deal breaker IMO, you just possibly get one redundant query when running validations. If anyone knows how to test if a has 1 is loaded though this can be fixed.

Anyone for see any issues with this? We have been using a monkey-patched version of this code on our app for a few months now (the bug caused much consternation before we identified it).

[gix]

So basically this is a different variant to get validation errors for associations? See https://github.com/solnic/dm-validations-ext (or a variant in my lang-agnostic branch: https://github.com/gix/dm-validations/commit/6449111088ef4f82019289b88eed554e7eb620df).

To validate associations I much prefer the approach by dm-validations-ext: not only do you get a correct #valid? response but also proper errors for associated models. Preventing unnecessary re-validation is a nice addition though.

IMO a mix of both should be merged into the next dm-validations.

[xaviershay]

Yeah they look to achieve the same goal (I didn't know about yours).

Yours gives a richer error model.
Mine checks for #loaded? status.
Mine allows validation for an association to be turned off. Not sure how useful this is, I figured it was safest to allow it.

I think I like the approach of adding an extra validation rather than hooking into the validate process itself, but not sure that would allow us the rich error model. Maybe the following would work:

if invalid?
  [false, children.map(&:errors)
end

I'll have to try.

[dkubb]

FYI I totally support this effort in combination with the error message reporting in dm-validations-ext.

Ideally we'd not lazy load anything (properties or associations) during validation, but I don't think that's a deal breaker. I'd rather see the changes made after 1.1.0 and then iteratively improved.

[skryl]

Are there any plans to merge either of the validation fixes into master? And will 1.3.0 ever get out of beta?