-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Bugfix collection association #create method #18155
Bugfix collection association #create method #18155
Conversation
Almost this exact change was introduced a few months back, but was reverted because it introduced another bug. I'll see if I can find it. |
@sgrif did you ever find that other big? |
@sgrif do you have some time to look into it again? |
It looks like this PR conflicts with master branch :) |
I'll look into it |
When same association is loaded in the model creation callback The new object is inserted into association twice
982f341
to
6d0d83a
Compare
rebased |
Welp, can't find what I was thinking of. If I actually did revert a change for causing problems, I expect that I would have added a test case. So ¯_(ツ)_/¯ |
…lement_fix Bugfix collection association #create method
… parent association saved in the callback Related rails#18155, rails#26661, 268a5bb, rails#27434, rails#27442, and rails#28599. Originally rails#18155 was introduced for preventing double insertion caused by the after save callback. But it was caused the before save issue (rails#26661). 268a5bb fixed rails#26661, but it was caused the performance regression (rails#27434). rails#27442 added new record to `target` before calling callbacks for fixing rails#27434. But it was caused double firing before save callback (rails#28599). We cannot add new object to `target` before saving the object. This is improving rails#18155 to only track callbacks after `save`. Fixes rails#28599.
When same association is loaded in the model creation callback
The new object is inserted into association twice.