From 2e3bba3e3af8c41e833cced61f0449ad73c947cc Mon Sep 17 00:00:00 2001
From: Ryan Kerr <leboshi@gmail.com>
Date: Sat, 30 Mar 2019 00:35:16 -0400
Subject: [PATCH] Fix callbacks on has_many :through associations (#33249)

When adding a child record via a has_many :through association,
build_through_record would previously build the join record, and then
assign the child record and source_type option to it.  Because the
before_add and after_add callbacks are called as part of build, however,
this caused the callbacks to receive incomplete records, specifically
without the other end of the has_many :through association.  Collecting
all attributes before building the join record ensures the callbacks
receive the fully constructed record.
---
 activerecord/CHANGELOG.md                      |  7 +++++++
 .../has_many_through_association.rb            | 14 ++++----------
 .../has_many_through_associations_test.rb      | 18 ++++++++++++++++++
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 690d487a09..21861ced31 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   Assign all attributes before calling `build` to ensure the child record is visible in
+    `before_add` and `after_add` callbacks for `has_many :through` associations.
+    
+    Fixes #33249.
+    
+    *Ryan H. Kerr*
+    
 *   Add `ActiveRecord::Relation#extract_associated` for extracting associated records from a relation. 
 
     ```
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index 84a9797aa5..b6049bdab4 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -57,21 +57,15 @@ module ActiveRecord
           @through_records[record.object_id] ||= begin
             ensure_mutable
 
-            through_record = through_association.build(*options_for_through_record)
-            through_record.send("#{source_reflection.name}=", record)
-
-            if options[:source_type]
-              through_record.send("#{source_reflection.foreign_type}=", options[:source_type])
-            end
+            attributes = through_scope_attributes
+            attributes[source_reflection.name] = record
+            attributes[source_reflection.foreign_type] = options[:source_type] if options[:source_type]
+            through_record = through_association.build(attributes)
 
             through_record
           end
         end
 
-        def options_for_through_record
-          [through_scope_attributes]
-        end
-
         def through_scope_attributes
           scope.where_values_hash(through_association.reflection.name.to_s).
             except!(through_association.reflection.foreign_key,
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 67e013c6e0..fa540c9dab 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -1474,6 +1474,24 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     assert_equal [subscription2], post.subscriptions.to_a
   end
 
+  def test_child_is_visible_to_join_model_in_add_association_callbacks
+    [:before_add, :after_add].each do |callback_name|
+      sentient_treasure = Class.new(Treasure) do
+        def self.name; "SentientTreasure"; end
+
+        has_many :pet_treasures, foreign_key: :treasure_id, callback_name => :check_pet!
+        has_many :pets, through: :pet_treasures
+
+        def check_pet!(added)
+          raise "No pet!" if added.pet.nil?
+        end
+      end
+
+      treasure = sentient_treasure.new
+      assert_nothing_raised { treasure.pets << pets(:mochi) }
+    end
+  end
+
   private
     def make_model(name)
       Class.new(ActiveRecord::Base) { define_singleton_method(:name) { name } }