Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

ActiveRecord::Associations::CollectionProxy#destroy_all raise exception instead of delete from database. #13085

Closed
GRoguelon opened this Issue · 9 comments

4 participants

@GRoguelon

Hi,

According the documentation, the method destroy_allshould remove the records directly from the database but when you have 3 models which have has_many association, an exception ActiveRecord::DeleteRestrictionErroris raised.

Take the example of a shop with categories and products:

class Shop < ActiveRecord::Base
  has_many :categories, inverse_of: :shop, dependent: :restrict_with_exception
end

class Category < ActiveRecord::Base
  belongs_to :shop, inverse_of: :categories

  has_many :products, inverse_of: :category, dependent: :restrict_with_exception
end

class Product < ActiveRecord::Base
  belongs_to :category
end

I have a test which reflect the problem:

class ShopTest < ActiveSupport::TestCase
 test 'shop can destroy if categories are blank' do
    assert_nothing_raised ActiveRecord::DeleteRestrictionError do
      shops(:apple).categories.destroy_all
      shops(:apple).destroy
    end
  end
end

The result of this test is:

ActiveRecord::DeleteRestrictionError: Cannot delete record because of dependent products

Normally, with the destroy_all method, the instruction shops(:apple).categories.destroy_allshould:

Deletes the records of the collection directly from the database. This will always remove the records ignoring the :dependent option.

I looked for briefly without success the cause of this problem but I would like have your opinion to be sure that is a bug.

I have created a demonstration application, you can download to GRoguelon/CollectionDestroyAll.

Thanks.

@rafaelfranca

Actually this is a documentation issue. destroy_all should use the :dependent option what should not is delete_all

@rafaelfranca

cc @neerajdotname could you confirm?

@GRoguelon

I will try to update my test with delete_all.

Thank you for your answers.

@rafaelfranca

@GRoguelon we didn't confirmed that this is the desirable behavior, so maybe it is still a bug. I'll update here shortly

@neerajdotname
Collaborator

@rafaelfranca yes destroy_all should respect :dependent option.

Also I think we should rephrase the sentence Deletes the records of the collection directly from the database.. In this sentence directly from the database leaves room for different interpretation.

@GRoguelon

Hi,

I've tested with delete_all and destroy_all. When this methods are used on a collection, I've noticed that delete_all method removes only the association between 2 models whereas `destroy_all removes the association and the records of the database. But the documentation says:

[...] This will always remove the records ignoring the :dependent option.

Thus, either the documentation is wrong and in this case, it seems to me that there is no way to remove an association and the records of the database. Or there is a bug on the destroy_allmethod because on 3 models like described in my first post, I get an ActiveRecord::DeleteRestrictionError exception.

Thanks.

@GRoguelon GRoguelon added the stale label
@rafaelfranca
Owner

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@GRoguelon

Hi,

I will try to reproduce this issue on the branches mentioned.

Thanks.

@rails-bot rails-bot closed this
@rails-bot
Collaborator

This issue has been automatically closed because of inactivity.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.