Rails Ruby on Rails → 3.x

Comments and changes to this ticket

• 

  Matt Jones May 14th, 2009 @ 08:03 PM

  There's an issue here, but your test does not trigger it. You'll need to save the Pirate record before bird.id will even have a value to use in the attribute call.

  I've attached a diff with a correct test; the fix is going to be more complicated.

  The issue is that, until the after_save callbacks from the nested_attributes code run, pirate.birds.size is 1. The bird instance has marked_for_destruction? set, but has not yet been destroyed.

  @jarl - in the interim, you might want to try a custom validation that only counts records with marked_for_destruction? false.

  Example:

  assert_nothing_raised do
    Pirate.validates_each(:birds) do |record, attr, value|
      record.errors.add attr, 'is too short.' if value.reject { |v| v.marked_for_destruction? }.size < 1
    end
  end
  

  or even define a custom attribute (a bit of a hack):

    Pirate.class_eval do
      def birds_not_destroyed
        birds.reject { |v| v.marked_for_destruction? }
      end
    end
  

  and run validates_size_of on that.

  • validation_on_records_to_be_destroyed.diff 1.4 KB
• 

  Michael Koziarski May 15th, 2009 @ 04:24 AM

  • Assigned user set to “Eloy Duran”

  Assigning to eloy as he's the dude here.

• You flagged this item as spam.
  

  Jarl Friis May 15th, 2009 @ 09:16 AM

  • Tag changed from “accepts_nested_attributes_for, associations, validation, validations” to “accepts_nested_attributes_for, active_record, associations, update_attributes, validation, validations”

  Thanks for taken my ticket serious.

  Actually the line in my test reading
  

  p.attributes = {:birds_attributes => [ {:_delete => 1, :id => bird.id}]}
  

  should have been
  

  p.update_attributes :birds_attributes => [ {:_delete => 1, :id => bird.id}]
  

  This was more like the code generated by scaffold that lead me to discover this problem, and as you have mentioned update_attributes do call save after the attributes=. Sorry for the confusion.

  @Matt: Thanks a lot for the temporary workaround you have suggested.

• You flagged this item as spam.
  

  Jarl Friis May 15th, 2009 @ 10:01 AM

  What ever the fix is going to be I think it is worth considering to make a fix so that p.birds.size should result in 0 after
  

  p.attributes = {:birds_attributes => [ {:_delete => 1, :id => bird.id}]}
  

  Basically that probably means that size has to be implemented to not count birds for which marked_for_destruction? is true. Remember that birds that is marked_for_destruction only still exists because the ids are need when p is saved to the database (so that the birds can be deleted). Alternatively is to store all birds that is marked_for_destruction into a different list which is then cleared when p is saved to the database.

• You flagged this item as spam.
  

  Jarl Friis May 15th, 2009 @ 02:51 PM

  @Matt: I see your point now. The test have to call save before the call to attributes=.

  Please ignore my comment @ 09:16 AM

• 

  Eloy Duran May 16th, 2009 @ 09:42 AM

  The dude has reserved some time at the end of the coming week to review problems and patches. :)

• 

  Eloy Duran July 12th, 2009 @ 01:04 PM

  I stil haven't been able to make up my mind on how to fix this cleanly. I'm currently leaning to overriding AssociationCollection #size & #length to ignore records that have been marked for destruction. Thoughts?

• 

  Michael Koziarski July 13th, 2009 @ 12:56 AM

  Wouldn't it be cleaner just to remove the records from the in memory array once they're marked for destruction. Then size and length will function as intended?

• 

  José Valim July 13th, 2009 @ 08:20 AM

  Good suggestion Koz, it would solve another problems as having destroyed records being validated when they shouldn't.

• 

  Matt Jones July 13th, 2009 @ 08:34 AM

  The only issue with removing them is that if some other validation fails, they'll need to be someplace so they can be returned back to the user.

  The most specific instance of this would be a validation that ensures that exactly N associated records exist (bad design, but it could happen). In that case, a user will have to delete some records if more are added, but the whole array will need to be around to display with the validation error.

• 

  Eloy Duran July 13th, 2009 @ 08:38 AM

  While I was typing an elaborate comment, Matt posted his, which says exactly what I wanted to say, but better :)

  So the problem is how to put them back after a failed validation, which might occur elsewhere then the current association, or an exception is raised etc. This might not be such a problem at all of course, I'd just have to look into it, but it seemed a way more complicated solution then having the collection return the size of what's actually going to be in the database.

• You flagged this item as spam.
  

  Adam Ingram-Goble January 2nd, 2010 @ 10:22 PM

  would it be possible to either move the marked_for_destruction records to another in memory array, or modify the association method so that it doesn't return the marked records by default? Either procedure allow for "rolling back" the failure. I think this is important for supporting the principle of least surprise.

• 

  Jeremy Kemper May 4th, 2010 @ 06:48 PM

  • Milestone changed from “2.x” to “3.x”

  [bulk edit]

• 

  Eloy Duran July 8th, 2010 @ 08:38 AM

  • Assigned user changed from “Eloy Duran” to “José Valim”
  • Importance changed from “” to “”
• 

  Neeraj Singh July 8th, 2010 @ 11:38 AM

  • Assigned user changed from “José Valim” to “Neeraj Singh”

  Did not know there were so many tickets related to nested_attributes. I will take a look at this one too.

• 

  Eloy Duran July 8th, 2010 @ 12:57 PM

  There aren't that many different ones though :)

  This is basically the same problem that comes up once in a while; how to make validates_size_of etc work with records marked for destruction. To use a separate array or not, that is the question…

• 

  Neeraj Singh July 29th, 2010 @ 03:28 AM

  • State changed from “new” to “open”

• You flagged this item as spam.
  

  wout August 19th, 2010 @ 09:41 AM

  I found out why associations sometimes aren't saved. If one of the database fields is changed validations are started and associations are saved, but if an attr_accessor value is changed not. Currently I add a hidden field to my nested forms setting :updated_at to Time.now and all is working fine again.

• 

  Ryan Bigg November 8th, 2010 @ 01:48 AM

  • Tag cleared.

  Automatic cleanup of spam.

• You flagged this item as spam.
  

  bingbing March 26th, 2011 @ 01:34 AM

  omega watch