This project is archived and is in readonly mode.

#2149 ✓invalid
Keith Pitt

Inconsistent has_many validation.

Reported by Keith Pitt | March 6th, 2009 @ 04:41 AM | in 2.x


class ModelA < ActiveRecord::Base
  has_many :model_bs
end
class ModelB < ActiveRecord::Base
  belongs_to :model_a
  validates_presence_of :name
end

I have 2 ActiveRecord model that look like the ones above. If I add an 'invalid' record to an existing record like this - the 'a' record becomes invalid.


  a = ModelA.create
  a.model_bs << ModelB.new :name => nil
  a.valid? # => false

However, if I do something like this:


  a = ModelA.create
  a.model_bs << ModelB.create
  a.valid? # => true
  a.model_bs.first.name = nil
  a.valid? # => true

the parent record is still valid even though I make an existing child record invalid. I have solved the problem by adding a:


  validates_associated :model_bs

but it seems a little odd to me to have an inconsistent default behaviour. Is this a bug or a feature?

Comments and changes to this ticket

  • Eloy Duran

    Eloy Duran March 6th, 2009 @ 10:13 AM

    Just to clarify a bit. Judging by the tag you are sure this was introduced in the first 2.3 release candidate?

  • Keith Pitt

    Keith Pitt March 6th, 2009 @ 11:18 PM

    I added that tag because thats the version I'm using (I thought that might be useful). But I haven't checked to see if its in the current stable version of rails.

  • Eloy Duran

    Eloy Duran March 7th, 2009 @ 11:52 AM

    • Tag changed from 2.3-rc1, activerecord to activerecord

    It would be very helpful if you could identify where (if) this was introduced.

    Thanks

  • Keith Pitt

    Keith Pitt March 7th, 2009 @ 12:38 PM

    Whats the best (fastest) way for me to do that?

  • Eloy Duran

    Eloy Duran March 7th, 2009 @ 12:49 PM

    However, if I do something like this:

    @@@ ruby a = ModelA.create a.model_bs << ModelB.create a.valid? # => true a.model_bs.first.name = nil a.valid? # => true @@@

    the parent record is still valid even though I make an existing child record invalid.

    Hmm, actually this might be because the target on the existing parent isn't loaded when using #push (See http://is.gd/mgf4).

    >

    If the target isn't loaded yet and you then use #first, it will not load the target. So that specific object you get returned is not going to be found by autosave association when iterating over any loaded records.

    You should be able to verify this by:

    • Not creating a parent object, so it's still new when #<< is called and the target will be loaded.
    • First load the target before using #first, because then #first will simply return from the loaded target. You can use this as a trick to make sure it's loaded: a.model_bs.class

    If it is this problem, then there's unfortunately no easier solution. However, I do found that using #first is something which is often only done in tests or script/console. Is that the case?

    I actually created a patch for this once, but it won't get applied in that state which I agree with. See #1751

  • Eloy Duran

    Eloy Duran March 7th, 2009 @ 12:54 PM

    Jeez, formatting is a pita. I edited it a few times, hope you can understand it.

  • Eloy Duran

    Eloy Duran March 7th, 2009 @ 12:58 PM

    Whats the best (fastest) way for me to do that?

    The fastest way to find where a bug was introduced, if it was, would be to use git-bisect. You create a file with a failing test and then do the whole git-bisect workflow and run the test each time to see where it/if it was introduced.

  • Eloy Duran

    Eloy Duran March 7th, 2009 @ 01:05 PM

    Oh and I forgot to add that it is weird that validates_associated would fix this…

  • José Valim

    José Valim March 8th, 2009 @ 04:57 PM

    This is very strange, because by default has_many validation was supposed to have :validate => true.

    Can you please try:

    has_many :model_bs, :validate => true

    And without the validates_associated?

    If you can still see a failure scenario, this is a serious bug, since validates_associated is behaving in a different way then :validate => true.

  • Keith Pitt

    Keith Pitt March 10th, 2009 @ 01:16 AM

    has_many :model_bs, :validate => true

    Tried that - but it still fails.

    I tried this, as per your thoughts Eloy ( hopefully I understood them correctly, your saying the problem is the parent not being loaded by just doing a #first? )

    
      a = ModelA.create
      a.model_bs << ModelB.create
      a.valid? # => true
      
      x = a.model_bs.first
      y = x.model_a
      x.name = nil
      y.valid? # => true
    

    But alas, still doesn't work as expected.

  • Keith Pitt

    Keith Pitt March 10th, 2009 @ 03:42 AM

    Here is my test script. I tested it with rails 2.2 (stable) and my tests fail. I went back to 2.1 but got more errors (not to do with me tests) - do I need to go back any further?

  • Eloy Duran

    Eloy Duran March 10th, 2009 @ 08:54 AM

    • Assigned user set to “Eloy Duran”
    • State changed from “new” to “invalid”

    I tested it with rails 2.2 (stable) and my tests fail. I went back to 2.1 but got more errors (not to do with me tests) - do I need to go back any further?

    That won't be necessary. Today I had some time to play with this (thanks for your test example) and it's pretty clear why this doesn't work. In the case of a collection association (such as a has_many) on a not new parent, only new associated records will be validated. (Also the default for a collection association is :validate => true, so setting that won't matter either.)

    So to answer your very first question: It's not a bug, it's a feature.

    The examples in which you expect that clearing the name of an existing Post will make the parent invalid won't work, as it simply skips that associated record. And trying to add new records with #<< won't work either, as they're automatically saved. (Use #build for that.)

    If you do want to always validate all associated records, you could enable :autosave. That way any loaded record will always get validated. However, they will also get saved together with the parent, but that might not be a problem in your situation. Keep in mind though that the problem about using #first still applies.

  • Eloy Duran

    Eloy Duran March 10th, 2009 @ 08:54 AM

    I tried this, as per your thoughts Eloy ( hopefully I understood them correctly, your saying the problem is the parent not being loaded by just doing a #first? )

    You didn't completely got my earlier comment, which is understandable as it can be strange/unexpected. What I mean is that you can't use #first or #last in this way unless the ‘target’ was loaded first by calling, for instance, #each on the collection. See #1752 for more info. But it won't matter in your case.

  • Ryan Bigg

    Ryan Bigg October 9th, 2010 @ 09:47 PM

    • Tag cleared.

    Automatic cleanup of spam.

  • bingbing

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Pages