Skip to content
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

Fix #857: Activerecord-Import imports associations for invalid models #858

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion lib/activerecord-import/import.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
22 changes: 22 additions & 0 deletions test/support/shared_examples/recursive_import.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down