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.

Sign up for GitHub

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

Jump to bottom

"after_commit on: :create" with "save" inside #32831

Closed
vfonic opened this issue May 6, 2018 · 1 comment
Closed

"after_commit on: :create" with "save" inside #32831

vfonic opened this issue May 6, 2018 · 1 comment

Comments

@vfonic
Copy link
Contributor

vfonic commented May 6, 2018 •
edited

This is the same issue like this one: after_commit :my_callback, :on => :create fires on update too

I'm posting a new issue, as the old one is closed. Should I just use the old issue? Is someone tracking closed issues' comments?

I've spent some time debugging to get to the bottom of this. I didn't know you can't call save or update_attributes! inside of an after_commit ..., on: :create callback.
I think this is a bug and it should be considered for fixing. I can see in the other thread that some rails maintainers also confirmed this is a bug.
There's this PR from 3 years ago that apparently fixes this, but the PR was never merged: #18367

@rafaelfranca said:

save inside a after_commit with on: :create is also inside the create lifecycle even if what it is doing is an update. The lifecycle only finish when the last after_commit callback runs, so we don't have a way to detect if that save is an update or not.

Not sure if we could have callback stack, like there is a function call stack. I'm just shooting an idea here.

How did it work before? This (revert) commit seems to revert the changes that introduced this bug. It seems like it's broken again.

System configuration

Rails version:
5.1.5
Ruby version:
2.4.2
@eugeneius
Copy link
Member

How did it work before? This (revert) commit seems to revert the changes that introduced this bug. It seems like it's broken again.

You have this backwards: #6226 fixed the bug, and the commit you linked to reverted the fix because it had other undesirable effects. As far as I can tell, this bug has always been present since after_commit callbacks were added to Rails.

@kamipo kamipo closed this as completed in ae02898 Jun 3, 2018
kamipo added a commit that referenced this issue Jun 4, 2018
@kamipo
Fix save in after_create_commit won't invoke extra `after_create_…
ddf7f10 
…commit`

Since a record is already persisted in `after_create_commit`, so `save`
should invoke only `after_update_commit`.

This bug is caused by depending on `@_start_transaction_state` for
rollback to consider whether it was `new_record` before being committed.

If after commit callbacks caused another commit, the state before
last commit is no longer `new_record`.

Fixes #32831.
Closes #18367.
Closes #31106.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@vfonic @eugeneius