Fix issue with destroy after RestartParentTransaction and SavepointTransaction rollback
Motivation / Background
This Pull Request has been created in order to fix issue #50338.
Detail
This Pull Request provides a working solution for the destroy function for the cases of RestartParentTransaction and SavepointTransaction.
Previously, the destroy function was not properly removing entries from the database for the RestartParentTransaction and SavepointTransaction. If @_trigger_destroy_callback is true at the beginning of the destroy function, destroy_row would never be invoked.
This Pull Request will ensure destroy_row is invoked when @_trigger_destroy_callback is true at the beginning of the destroy function. This will remove the corresponding row from the database. Otherwise, if @_trigger_destroy_callback is false, destroy_row is only invoked if persisted? is true.
Additional information
Checklist
Before submitting the PR make sure the following are checked:
- [x] This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
- [x] Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex:
[Fix #issue-number] - [x] Tests are added or updated if you fix a bug or add a feature.
- [x] CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.
I suggest a fix to your branch, here is the pull request to review: https://github.com/mguan2020/rails/pull/1
@rubyrider Thank you very much for the fix! I have merged your fix into this PR.
thanks!
@rubyrider I have squashed my changes into a single commit.
@rubyrider I have squashed my changes into a single commit.
I think keeping that commit would give him co author contributing credit?
@auvipy thank you for your concern dear, but this is totally fine for me, I don't need a credit 👍 I am here as a volunteer purpose only.
@auvipy Thank you for your idea. I am following the Contributing to Ruby on Rails guide, which states that commits should be squashed for cleaner commit history.
I am really thankful for @rubyrider contributions to this PR and believe he should be listed as a co-author. It is very nice that we worked on this together.
I notice the build has failed. The reason seems to be from a failed test case in strict_loading_test.rb. I am a bit confused, as the command bundle exec ruby -Itest test/cases/strict_loading_test.rb passes with no errors.
Is there anything I'm missing?
@mguan2020 i will take a look.
@mguan2020 its passing locally. can you try re-running the test or rebase your branch and update your branch to see if the latest build fixes it?
Thank you @rubyrider for your response. I reran the test and it is also passing locally on my end. I followed your advice of rebasing and updating my branch.
After updating my feature branch and running all the tests in Active Record, I am getting the following output.
Finished in 183.864905s, 47.2031 runs/s, 160.3841 assertions/s. 8679 runs, 29489 assertions, 0 failures, 0 errors, 33 skips
Could you give me an update on how things are going?
I'm sorry for asking multiple times. Could you give me an update on how things are going?