Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

restrict_with_error raises exception when destroying CollectionProxy #35298

Closed
shanecav84 opened this issue Feb 16, 2019 · 7 comments
Closed
Labels

Comments

@shanecav84
Copy link

Steps to reproduce

Executable Test Case

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "activerecord", "5.2.2"
  gem "sqlite3", "~> 1.3.6"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :posts, force: true do |t|; end

  create_table :comments, force: true do |t|
    t.integer :post_id
  end

  create_table :sub_comments, force: true do |t|
    t.integer :comment_id
  end
end

class Post < ActiveRecord::Base
  has_many :comments
  accepts_nested_attributes_for :comments, allow_destroy: true
end

class Comment < ActiveRecord::Base
  belongs_to :post
  has_many :sub_comments, dependent: :restrict_with_error
end

class SubComment < ActiveRecord::Base
  belongs_to :comment
end

class BugTest < Minitest::Test
  def test_association_stuff
    post = Post.create!
    comment = Comment.create!(post: post)
    SubComment.create!(comment: comment)

    invalid = assert_raises ::ActiveRecord::RecordNotDestroyed do
      post.update(comments_attributes: [{ id: comment.id, _destroy: true }])
      # `post.comments.destroy_all` also applies
    end
    assert_equal invalid.record, comment
    assert_equal invalid.record.errors.full_messages,
      ["Cannot delete record because dependent sub comments exist"]
  end
end

Expected behavior

To not raise exception due to restrict_with_error and non-bang update.

Actual behavior

Raises exception like restrict_with_exception would despite restrict_with_error.

System configuration

Rails version: 5.2.2

Ruby version: 2.6.1

@shanecav84
Copy link
Author

shanecav84 commented Feb 16, 2019

Similar issues for previous versions of Rails: #20991, #24526

@shanecav84
Copy link
Author

Here is the backtrace from the ActiveRecord::RecordNotDestroyed exception:

ActiveRecord::RecordNotDestroyed: Failed to destroy the record
    activerecord-5.2.2/lib/active_record/persistence.rb:748:in `_raise_record_not_destroyed'
    activerecord-5.2.2/lib/active_record/persistence.rb:355:in `destroy!'
    activerecord-5.2.2/lib/active_record/associations/has_many_association.rb:107:in `each'
    activerecord-5.2.2/lib/active_record/associations/has_many_association.rb:107:in `delete_records'
    activerecord-5.2.2/lib/active_record/associations/collection_association.rb:400:in `remove_records'
    activerecord-5.2.2/lib/active_record/associations/collection_association.rb:393:in `block in delete_or_destroy'
    activerecord-5.2.2/lib/active_record/associations/collection_association.rb:136:in `block in transaction'
    activerecord-5.2.2/lib/active_record/connection_adapters/abstract/database_statements.rb:257:in `transaction'
    activerecord-5.2.2/lib/active_record/transactions.rb:212:in `transaction'
    activerecord-5.2.2/lib/active_record/associations/collection_association.rb:135:in `transaction'
    activerecord-5.2.2/lib/active_record/associations/collection_association.rb:393:in `delete_or_destroy'
    activerecord-5.2.2/lib/active_record/associations/collection_association.rb:199:in `destroy'
    activerecord-5.2.2/lib/active_record/autosave_association.rb:391:in `block in save_collection_association'
    activerecord-5.2.2/lib/active_record/autosave_association.rb:391:in `each'
    activerecord-5.2.2/lib/active_record/autosave_association.rb:391:in `save_collection_association'
    activerecord-5.2.2/lib/active_record/autosave_association.rb:187:in `block in add_autosave_association_callbacks'
    activerecord-5.2.2/lib/active_record/autosave_association.rb:159:in `instance_eval'
    activerecord-5.2.2/lib/active_record/autosave_association.rb:159:in `block in define_non_cyclic_method'
    activesupport-5.2.2/lib/active_support/callbacks.rb:426:in `block in make_lambda'
    activesupport-5.2.2/lib/active_support/callbacks.rb:236:in `block in halting_and_conditional'
    activesupport-5.2.2/lib/active_support/callbacks.rb:517:in `block in invoke_after'
    activesupport-5.2.2/lib/active_support/callbacks.rb:517:in `each'
    activesupport-5.2.2/lib/active_support/callbacks.rb:517:in `invoke_after'
    activesupport-5.2.2/lib/active_support/callbacks.rb:133:in `run_callbacks'
    activesupport-5.2.2/lib/active_support/callbacks.rb:816:in `_run_update_callbacks'
    activerecord-5.2.2/lib/active_record/callbacks.rb:350:in `_update_record'
    activerecord-5.2.2/lib/active_record/timestamp.rb:114:in `_update_record'
    activerecord-5.2.2/lib/active_record/persistence.rb:703:in `create_or_update'
    activerecord-5.2.2/lib/active_record/callbacks.rb:342:in `block in create_or_update'
    activesupport-5.2.2/lib/active_support/callbacks.rb:132:in `run_callbacks'
    activesupport-5.2.2/lib/active_support/callbacks.rb:816:in `_run_save_callbacks'
    activerecord-5.2.2/lib/active_record/callbacks.rb:342:in `create_or_update'
    activerecord-5.2.2/lib/active_record/persistence.rb:273:in `save'
    activerecord-5.2.2/lib/active_record/validations.rb:46:in `save'
    activerecord-5.2.2/lib/active_record/transactions.rb:310:in `block (2 levels) in save'
    activerecord-5.2.2/lib/active_record/transactions.rb:387:in `block in with_transaction_returning_status'
    activerecord-5.2.2/lib/active_record/connection_adapters/abstract/database_statements.rb:257:in `transaction'
    activerecord-5.2.2/lib/active_record/transactions.rb:212:in `transaction'
    activerecord-5.2.2/lib/active_record/transactions.rb:385:in `with_transaction_returning_status'
    activerecord-5.2.2/lib/active_record/transactions.rb:310:in `block in save'
    activerecord-5.2.2/lib/active_record/transactions.rb:325:in `rollback_active_record_state!'
    activerecord-5.2.2/lib/active_record/transactions.rb:309:in `save'
    activerecord-5.2.2/lib/active_record/suppressor.rb:44:in `save'
    activerecord-5.2.2/lib/active_record/persistence.rb:426:in `block in update'
    activerecord-5.2.2/lib/active_record/transactions.rb:387:in `block in with_transaction_returning_status'
    activerecord-5.2.2/lib/active_record/connection_adapters/abstract/database_statements.rb:259:in `block in transaction'
    activerecord-5.2.2/lib/active_record/connection_adapters/abstract/transaction.rb:239:in `block in within_new_transaction'
    monitor.rb:230:in `mon_synchronize'
    activerecord-5.2.2/lib/active_record/connection_adapters/abstract/transaction.rb:236:in `within_new_transaction'
    activerecord-5.2.2/lib/active_record/connection_adapters/abstract/database_statements.rb:259:in `transaction'
    activerecord-5.2.2/lib/active_record/transactions.rb:212:in `transaction'
    activerecord-5.2.2/lib/active_record/transactions.rb:385:in `with_transaction_returning_status'
    activerecord-5.2.2/lib/active_record/persistence.rb:424:in `update'
    ar_destroy_bug.rb:55:in `test_association_stuff'

@shanecav84
Copy link
Author

It looks like this is not a bug but is intended behavior. From collection_association.rb:

# Deletes the +records+ and removes them from this association calling
# +before_remove+ , +after_remove+ , +before_destroy+ and +after_destroy+ callbacks.
#
# Note that this method removes records from the database ignoring the
# +:dependent+ option.
def destroy(*records)
  delete_or_destroy(records, :destroy)
end

Why is the dependent option ignored here?

@rails-bot
Copy link

rails-bot bot commented May 20, 2019

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 5-2-stable branch 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.

@cimm
Copy link

cimm commented May 29, 2020

I seem to have the same issue in Rails 6.0.2.

Using restrict_with_error raises an ActiveRecord::RecordNotDestroyed error when updating the nesting model while restrict_with_exception raises an ActiveRecord::DeleteRestrictionError error.

The second one makes sense but the first one should not raise an exception IMHO.

@DaAwesomeP
Copy link

@cimm did you find a workaround?

@cimm
Copy link

cimm commented Nov 14, 2023

Hi @DaAwesomeP, sorry this has been years ago, I no longer remember if or how I handled this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants