On destroying do not touch destroyed belongs to association. #13455
Conversation
| invoice = Invoice.create!(line_items: [line_item]) | ||
|
|
||
| # 2 queries with delete of invoce and line_time records | ||
| # and 1 query for update `update_at` column for invoice |
There was a problem hiding this comment.
I don't think the extra comments are needed.
|
Thanks guys, updated! |
| @@ -1,3 +1,30 @@ | |||
| * Do not raise `'can not touch on a new record object'` exception on destroying already destroyed | |||
| object with touch association causes | |||
There was a problem hiding this comment.
@pftg I think you left the "causes" by mistake post editing, or maybe I am reading the sentence wrong.
| comment.destroy # => raised exception on touching destroyed record | ||
|
|
||
| #After | ||
| comment.destroy # => does not touch destroyed record |
There was a problem hiding this comment.
The fix is quite obvious from your description, I think we can shrink the example code to the minimum:
# Given Comment has belongs_to :post, touch: true
comment.post.destroy
comment.destroy # no longer raises an error
|
@senny thanks for review, updated! |
| line_item = LineItem.create! | ||
| invoice = Invoice.create!(line_items: [line_item]) | ||
|
|
||
| assert_queries(1) { invoice.destroy } |
There was a problem hiding this comment.
no need to assert_queries here. this should be considered setup code.
|
@pftg can you push a rebased version, this does no longer apply. |
|
@senny done! |
…record On destroying do not touch destroyed belongs to association.
|
@senny thanks! |
|
@senny, I'm not clear what did you mean, but this bug appears for One thing only, that for Should I add new PR for |
|
basically the exception raising behavior is only on master. However you are right, that the unnecessary touch query is executed. I can backport a modofied version of this oatch with an updated changelog entry. No need for another PR. |
|
@senny you are right. Have refactored test to make it raise exception: def test_belongs_to_with_touch_option_on_destroy_with_destroyed_parent
invoice = Invoice.create!
line_item = LineItem.create!(invoice: invoice)
invoice.destroy
assert_queries(1) { line_item.destroy }
end |
…record On destroying do not touch destroyed belongs to association. Conflicts: activerecord/CHANGELOG.md activerecord/lib/active_record/associations/builder/belongs_to.rb The code that raises an error when touching destroyed records is not on `4-0-stable`. It's only on `master`: e52ff80 I modified the CHANGELOG entry to explain the bug rather the exception raising behavior. I also had to modify the test-case to use the same instance. Otherwise `destroy` did not result on a `destroyed?` association record. /cc @pftg
Fixes: #13445