Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• You flagged this item as spam.
  

  Jon Dahl August 19th, 2009 @ 03:45 PM

  Big thanks for turning this into a "real" patch, Mike.

  Two small comments.

  First, the "else" path should be modified to list the :restrict option:

  raise ArgumentError, "The :dependent option expects either :destroy, :delete, :restrict, or :nullify (#{reflection.options[:dependent].inspect})"

  Second, is ActiveRecord::StatementInvalid the right exception to raise here? I threw that in to my sample code just because it's what would be raised by a DB foreign key that restricted deletes. But this isn't technically an invalid SQL statement. Is it worth creating a new exception class for this?

• 

  Pratik August 19th, 2009 @ 03:49 PM

  Yup we should have a new exception here.

• You flagged this item as spam.
  

  Jon Dahl August 19th, 2009 @ 04:14 PM

  I'll test the patch on an app today, and add a new exception - ActiveRecord::DependentRecordError or something like that.

• 

  CancelProfileIsBroken August 19th, 2009 @ 04:59 PM

  Here's the revised patch

  • restrict2.diff 9.2 KB
• 

  Xavier Noria August 21st, 2009 @ 06:40 PM

  Good, there are common use cases for this. Just some minor details:

  • The title of the ticket itself reads :restricted :-).

  • There are a couple of places where docs say "If the association is marked as :dependent => :restrict, create a callback that prevents deleting entirely.". That's true if there exists children, otherwise deletion is performed.

  • I think it could be good to document ActiveRecord::DeleteRestrictionError somewhere, because now destroy may throw it.

  • I'd prefer not to load the parent record in has_one to check if it exists, but there is no API for that it would need to be done by hand I guess. (Just a remark... no sure it is worth the trouble, I have in mind exists == @target || select 1 or something in that line).

  • There's also a case in HasOneAssociation#replace that I think should say something about :restrict. I don't really remember what's the purpose of #replace. The premise is that child records are untouchable, so an exception would make sense. You are not deleting the parent though, but you do not have information about how to destroy the child either. I think what to do is not completly clear, but I believe raising the exception is sound.

• 

  CancelProfileIsBroken September 25th, 2009 @ 12:49 PM

  • Tag set to “bugmash”
  • Assigned user cleared.

• You flagged this item as spam.
  

  Elad Meidar September 26th, 2009 @ 07:52 AM

  +1 on idea but patch (v2) does not apply on neither master or 2-3-stable. i've attached a patch for both, please note that i had no idea how to credit mike for the entire thing, so my name is up there should be changed to mike before committing

  @xavier, your 4th point is something that i having running around in the past few days.. the fact that there is no way to access an associated model from within a reflection / association scope. There are many tickets that share this basic problem, this #3027 ticket would be easily solved if there was any kind of access to the associated model (and therefore AR::Base.table_name).
  Solving this issue can clear some significant amount of tickets.

  • LH83075-dependent_restrict_for_has_one_or_many_2-3-stable.diff 8.9 KB
  • LH3075-dependent_restrict_for_has_one_or_many_master.diff 8.7 KB

• You flagged this item as spam.
  

  Elad Meidar September 26th, 2009 @ 07:52 AM

  • Tag changed from “bugmash” to “bugmash, patch”
• 

  Rizwan Reza January 21st, 2010 @ 07:17 AM

  • Milestone cleared.
  • Tag changed from “bugmash, patch” to “activerecord, dependent, patch”
  • State changed from “new” to “open”
  • Assigned user set to “Pratik”
• 

  Rizwan Reza March 28th, 2010 @ 07:08 PM

  • State changed from “open” to “resolved”

  (from [f1e5a9ff982998c31c9dedf2fa79180c7dea820a]) Add :dependent = to has_one and has_many [#3075 state:resolved] http://github.com/rails/rails/commit/f1e5a9ff982998c31c9dedf2fa7918...

• 

  Jeremy Kemper October 15th, 2010 @ 11:01 PM

  • Milestone set to “3.0.2”
  • Importance changed from “” to “Low”

  [bulk edit]