From 39f93e2e8d110433f866cb55e868225ebeca1bb6 Mon Sep 17 00:00:00 2001 From: Jacob Carlborg Date: Tue, 5 Nov 2024 10:29:10 +0100 Subject: [PATCH] Fix #857: Activerecord-Import imports associations for invalid models --- CHANGELOG.md | 4 ++++ lib/activerecord-import/import.rb | 3 ++- .../shared_examples/recursive_import.rb | 22 +++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e1f42b2..2993a18e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ associations and class names, i.e. when the `class_name:` option is used on an association. +### Fixes + +* Activerecord-Import imports associations for invalid models. Thanks to @jacob-carlborg-apoex via \##858. + ## Changes in 1.8.1 ### Fixes diff --git a/lib/activerecord-import/import.rb b/lib/activerecord-import/import.rb index d7eb92b6..16289d65 100644 --- a/lib/activerecord-import/import.rb +++ b/lib/activerecord-import/import.rb @@ -746,7 +746,8 @@ def import_helper( *args ) # if there are auto-save associations on the models we imported that are new, import them as well if options[:recursive] options[:on_duplicate_key_update] = on_duplicate_key_update unless on_duplicate_key_update.nil? - import_associations(models, options.dup.merge(validate: false)) + valid_models = models.filter { |model| model.errors.blank? } + import_associations(valid_models, options.dup.merge(validate: false)) end end diff --git a/test/support/shared_examples/recursive_import.rb b/test/support/shared_examples/recursive_import.rb index 02274315..25642d5c 100644 --- a/test/support/shared_examples/recursive_import.rb +++ b/test/support/shared_examples/recursive_import.rb @@ -151,6 +151,28 @@ def should_support_recursive_import end end + context "when a model is not valid" do + # The issue [1] only occurs when the parent model has an ID set. + # [1] https://github.com/zdennis/activerecord-import/issues/857 + let(:invalid_topics) { Build(1, :topic_with_book, author_name: nil, id: 1) } + + context "when validations are enabled" do + it "does not import its associations" do + assert_no_difference "Book.count" do + Topic.import invalid_topics, validate: true, recursive: true + end + end + end + + context "when validations are not enabled" do + it "import its associations anyway" do + assert_difference "Book.count", invalid_topics.first.books.size do + Topic.import invalid_topics, validate: false, recursive: true + end + end + end + end + unless ENV["SKIP_COMPOSITE_PK"] describe "with composite primary keys" do it "should import models and set id" do