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

Fix create callback called on destroy #513

Conversation

bkDJ
Copy link
Contributor

@bkDJ bkDJ commented Sep 17, 2021

We were running into a weird bug in one of our projects while migrating from Rails 5.2 to Rails 6.0. It seems to be due to Rails using #with_transaction_returning_status when persisting models which takes care of setting the recently-added ivar @_new_record_before_last_commit. Paranoia overrides ActiveRecord's #destroy with #paranoia_destroy which unfortunately only uses #transaction, so when checking for which callbacks should be run, it was seeing a stale value of @_new_record_before_last_commit, therefore causing after_create_commit hooks to be called again.

@mathieujobin
Copy link
Collaborator

can you please rebase your branch with core so the tests run?
Thanks

@rchasman
Copy link

Would really appreciate getting this fix into the wild :)

@arlando
Copy link

arlando commented Jan 18, 2022

@mathieujobin is the requirement just rebasing the branch to core and this will be GTG? thnx

@mathieujobin
Copy link
Collaborator

I will have another review after I see all tests passing
But it looked fine from what I remember.

Copy link
Collaborator

@mathieujobin mathieujobin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just would like to see the tests running on CI ...

@mathieujobin
Copy link
Collaborator

I have rebased this branch into a new branch. all tests passed. including with the test in #522

I will release later

https://github.com/rubysherpas/paranoia/actions/runs/2026193146

@mathieujobin mathieujobin merged commit cd60ab2 into rubysherpas:core Mar 23, 2022
karunkumar1ly pushed a commit to edcast/paranoia that referenced this pull request Feb 6, 2024
* update Rails

* add specs for broken case

* apply fix

Co-authored-by: Djilani Kebaili <djilani.kebaili@epfl.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants