[PATCH] belongs_to should take notice of the :dependent option

-1

I don't think that's a very good idea, a belongs_to relationship has a has_many relationship on the other end. I don't see why one would want to destroy the object it belong to.

I might have missed your point though.

The :dependent option should not be used with a has_many on the other end. My use case is the following:

class Person < ActiveRecord::Base
  belongs_to :address, :dependent => :destroy
end

class Address < ActiveRecord::Base
end

It seems perfectly reasonable to me.

Replying to jonathan_viney:

The :dependent option should not be used with a has_many on the other end. My use case is the following: {{{ class Person < ActiveRecord::Base belongs_to :address, :dependent => :destroy end class Address < ActiveRecord::Base end }}} It seems perfectly reasonable to me.

What if you had something like that:

class Person < ActiveRecord::Base
  belongs_to :address, :dependent => :destroy
end

class Address < ActiveRecord::Base
  has_many :people
end

Many people would live at the same address, if I now delete one person, the whole address gets destroyed? That doesn't make sense to me.

I think you should be using the has_one relationship and by doing so, you'd get the :dependent => :destroy and :dependent => :nullify options you're after.

Unless you have a compelling case against the has_one, I think this modification isn't valid.

Replying to matt:

What if you had something like that: {{{ class Person < ActiveRecord::Base belongs_to :address, :dependent => :destroy end class Address < ActiveRecord::Base has_many :people end }}} Many people would live at the same address, if I now delete one person, the whole address gets destroyed? That doesn't make sense to me.

But why would you specify :dependent there? You're specifying behaviour you don't want and then then complaining when it happens!

I think you should be using the has_one relationship and by doing so, you'd get the :dependent => :destroy and :dependent => :nullify options you're after. Unless you have a compelling case against the has_one, I think this modification isn't valid.

I have some models that have two addresses, and I think I'm right in saying that you can not have two has_ones to the same table. Eg:

class Person < ActiveRecord::Base
  belongs_to :street_address, :class_name => "Address", :dependent => :destroy
  belongs_to :mailing_address, :class_name => "Address", :dependent => :destroy
end

class Address < ActiveRecord::Base
end

How does that translate nicely into using has_one instead of belongs_to?

Can we get some other people's input on this?

Looks good to me.

@matt, if you had set things up this way, you could have a foreign key constraint restricting its deletion.

my bad, I didn't think about the second case.

However, I still think it's a bit dangerous to go this way, not that we shouldn't do it but maybe have an extra warning in the doc or something.

Sorry Jonathan for the misunderstanding.

All good. I think a warning in the docs is a good idea.

Replying to matt:

my bad, I didn't think about the second case. However, I still think it's a bit dangerous to go this way, not that we shouldn't do it but maybe have an extra warning in the doc or something. Sorry Jonathan for the misunderstanding.

Added patch with docs mentioning that dependent should not be used with belongs_to/has_many.

I'd agree that belongs_to :dependent would be a dangerous idea.

With regard to jonathan_viney, I'd suggest the following:

class Person < ActiveRecord::Base
  with_options :class_name => "Address", :dependent => :destroy do |addresses|
    addresses.has_one :street_address, :conditions => {:type => "street"}
    addresses.has_one :mailing_address, :conditions => {:type => "mailing}
  end
end

class Address < ActiveRecord::Base
  belongs_to :person
  validates_inclusion_of :type, :in => %w(street mailing)
end

That's untested, and there may be a few gotchas (create_street_address probably wouldn't automatically assign type, for instance), but it would achieve what you want, and I feel that :dependent really only belongs on has_one and has_many. Perhaps this could be implemented as a shortcut (some metaprogramming), but it seems like such an edge case that even that may not be worth the effort.

Any option can be used in the wrong place. Placing a :dependent option on any relationship that should not be dependent will result in incorrect behaviour, that's just common sense. If you ask for behaviour you do not want, you shouldn't be surprised when you get it! I can see that some people may incorrectly use it with a belongs_to/has_many scenario, so the documentation mentions that it should not be used in this case.

The natural solution for my situation is belongs_to :dependent. I'm sure there are many ways which I could hack in the required behaviour, but that does not seem like a good solution.

Replying to manitoba98:

I'd agree that belongs_to :dependent would be a dangerous idea. With regard to jonathan_viney, I'd suggest the following:

I have to say I like this idea. :dependent => :destroy denotes that it is a somewhat dangerous method, if you don't want your child referenced rows to be wacked don't set this value. At least that is my thinking. I have needed this a few times to keep orphan rows from sticking around.

+1

could you update the patch for trunk?

Sure thing.

(In [8675]) belongs_to supports :dependent => :destroy and :delete. Closes #10592.

changeset 8675 doesn't include the author_addresses.yml file. This makes the test fail. I attached a new patch that creates the table, which makes the test succeed.

(In [8682]) Add missing author_addresses.yml fixture. Closes #10592.