Merge pull request #852 from jacob-carlborg-apoex/non-standard-associ…

…ation-843

Fix #843: recursive_on_duplicate_key_update doesn't work with non-standard association name

CHANGELOG.md

@@ -1,3 +1,17 @@
+## Unreleased
+
+### Breaking Changes
+
+* Fix `recursive_on_duplicate_key_update` doesn't work with non-standard
+  association name. The documentation for the
+  `:recursive_on_duplicate_key_update` option specifies that the hash key is
+  the association name. But previously the name of associated table was used to
+  look up the options. Now the behavior matches the documentation and the name
+  of the association is used instead. This only affects associations that uses
+  a name that doesn't follow the ActiveRecord naming conventions of
+  associations and class names, i.e. when the `class_name:` option is used on
+  an association.
+
 ## Changes in 1.8.1
 
 ### Fixes

lib/activerecord-import/import.rb

@@ -857,12 +857,11 @@ def import_without_validations_or_callbacks( column_names, array_of_attributes,
 
     private
 
-    def associated_options(options, associated_class)
+    def associated_options(options, association)
       return options unless options.key?(:recursive_on_duplicate_key_update)
 
-      table_name = associated_class.arel_table.name.to_sym
       options.merge(
-        on_duplicate_key_update: options[:recursive_on_duplicate_key_update][table_name]
+        on_duplicate_key_update: options[:recursive_on_duplicate_key_update][association]
       )
     end
 
@@ -971,12 +970,12 @@ def import_associations(models, options)
       options.delete(:returning)
 
       associated_objects_by_class.each_value do |associations|
-        associations.each_value do |associated_records|
+        associations.each do |association, associated_records|
           next if associated_records.empty?
 
           associated_class = associated_records.first.class
           associated_class.bulk_import(associated_records,
-                                       associated_options(options, associated_class))
+                                       associated_options(options, association))
         end
       end
     end

test/models/topic.rb

@@ -16,6 +16,7 @@ class Topic < ActiveRecord::Base
   before_validation -> { errors.add(:title, :invalid) if title == 'invalid' }
 
   has_many :books, inverse_of: :topic
+  has_many :novels, inverse_of: :topic, class_name: "Book"
   belongs_to :parent, class_name: "Topic"
 
   composed_of :description, mapping: [%w(title title), %w(author_name author_name)], allow_nil: true, class_name: "TopicDescription"

test/support/shared_examples/recursive_import.rb

@@ -278,6 +278,45 @@ def should_support_recursive_import
           end
           assert_equal new_chapter_title, Chapter.find(example_chapter.id).title
         end
+
+        context "when a non-standard association name is used" do
+          let(:new_topics) do
+            topic = Build(:topic)
+
+            2.times do
+              novel = topic.novels.build(title: FactoryBot.generate(:book_title), author_name: 'Stephen King')
+              3.times do
+                novel.chapters.build(title: FactoryBot.generate(:chapter_title))
+              end
+
+              4.times do
+                novel.end_notes.build(note: FactoryBot.generate(:end_note))
+              end
+            end
+
+            [topic]
+          end
+
+          it "updates nested associated objects" do
+            new_chapter_title = 'The Final Chapter'
+            novel = new_topics.first.novels.first
+            novel.author_name = 'Richard Bachman'
+
+            example_chapter = novel.chapters.first
+            example_chapter.title = new_chapter_title
+
+            assert_nothing_raised do
+              Topic.import new_topics,
+                recursive: true,
+                on_duplicate_key_update: [:id],
+                recursive_on_duplicate_key_update: {
+                  novels: { conflict_target: [:id], columns: [:author_name] },
+                  chapters: { conflict_target: [:id], columns: [:title] }
+                }
+            end
+            assert_equal new_chapter_title, Chapter.find(example_chapter.id).title
+          end
+        end
       end
     end
 

test/test_helper.rb

@@ -13,7 +13,10 @@
 require "bundler"
 Bundler.setup
 
-require 'pry' unless RbConfig::CONFIG["RUBY_INSTALL_NAME"] =~ /jruby/
+unless RbConfig::CONFIG["RUBY_INSTALL_NAME"] =~ /jruby/
+  require 'pry'
+  require 'pry-byebug'
+end
 
 require "active_record"
 require "active_record/fixtures"