Association after_add callback for new records

rubyonrails-core
1

Hi everyone

Please help me to identify the issue here. Either with the code or the documentation, not sure where the issue lies at the moment. Code first, then question.

I have replicated the issue with this simple test on edge:

activerecord/test/cases/associations/callback_test.rb: def test_building_macros_callbacks post = @david.posts_with_callbacks.build( :title => ‘Work in progress’ ) assert post.new_record? assert_equal [“before_adding”], @david.post_log # ← ASSERTION FAILS HERE

assert post.save
@david.equal ["before_adding<new>", "after_adding#{[post.id](http://post.id)}"], @david.post_log

end

Test failure: <[“before_adding”]> expected but was <[“before_adding”, “after_adding”]>.

I have some functionality that depends on #after_add only getting called when the new association member is saved. The docs doesn’t state what would happen with new records, but my logic assumed that #after_add would only be called once the new association member has been saved. I suspect the later holds, cause it is only then when it truly becomes part of the association…

Now, do I fix the docs and work on another solution, or do I attempt to change the behavior to the one outlined above?

Kind regards

2

Hi Kenneth,

Could you try if this regression was introduced after this commit: 5cda000bf0f6d85d1a1efedf9fa4d0b6eaf988a1

Cheers,

Eloy

3

Hi Eloy

Thanks, went back to dec91a2e0655d288bd5184c981e2f4c60d9b3cf1 (one previous to the one you suggested) and the bug is still visible. I also branched off the v2.2.2 tag, and the bug is visible back then as well.

I’ll try my best blame/bisect combos to see if I can find the offending commit that broke this, seems to be way older than thought.

Kind regards

4

Thanks for investigating Kenneth! At least now we know I didn’t break this for 2.3 RC2 :slight_smile:

Eloy

5

This getting very strange now… Went back all the way to 2.0.5, and it still didn’t work. So I git-log’ed the callback test case to find its “origin” as a starting point.

This lead me to r9224 (36b8073ff553e) and ticket #11374 (http://dev.rubyonrails.org/ticket/11374). The test still fails here. At this stage I’m beginning to wonder if it ever really work at all…

So I jumped back one commit, to r9223 (50538fb5249), and the test still fails.

Should I carry on with this trip down memory lane, or branch off and attempt to fix it as I mentioned in my first email?

Best

6

Looking closely at the patch provided on #11374 I see the test was probably just copied from somewhere else where the behavior was already specified to call #after_add before saving the child association.

So, this makes me think that I inferred the wrong behavior from the documentation. But I also want to add that I strongly feel that #after_add should only be called once the association is binded permanently in the database, since that is the behavior of all the other ActiveRecord callbacks.

Any thoughts?

7

I would go for the latter at this point.

Eloy

8

Cool, I’ll kick off an attempt to get the expected behavior. If anyone wants to stop me, please do it quickly :slight_smile:

Best

9

http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/2100-patch-change-behaviour-of-associations-after_add-callback

(Starting a new thread)

10

OK, some feedback as to where I’ve gotten with this one.

Ticket: http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/2100-patch-change-behaviour-of-associations-after_add-callback

I gave some sample code in the ticket, and it’s part of the second patch, which highlights an issues with has_many :through’s and the association callbacks. The after_add callback gets called, but the association is never setup correctly. I’m not exactly sure how to proceed with this one now and need some advice. Did I blow open another issue here?

Best