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

has_many associations don't save with recursive: :true #247

Closed
rhannequin opened this issue Mar 2, 2016 · 21 comments
Closed

has_many associations don't save with recursive: :true #247

rhannequin opened this issue Mar 2, 2016 · 21 comments

Comments

@rhannequin
Copy link

Hello,

First, thank you very much for this gem, it is much appreciated every day from casual to complex data imports.

I am having troubles with the multi-level feature, when trying to import models with associations. I tried to follow the multi-level example this way:

I created a blank app with two models:

$ bundle exec rails g model Book title:string
$ bundle exec rails g model Review name:string book:references

Then I added the association to the Book model:

class Book < ActiveRecord::Base
  has_many :reviews
end

I created a basic rake task with the same code as the example, to insert books with one review in each, and checking at the end if everything went good:

task test: :environment do
  Book.delete_all
  Review.delete_all

  books = []
  10.times do |i|
    book = Book.new(title: "Book ##{i}")
    book.reviews.build(name: "Review ##{i}")
    books << book
  end
  Book.import books, recursive: true

  puts "'Book' records count: #{Book.count}"
  puts "Last 'Book' reviews count: #{Book.last.reviews.count}"
  puts "'Review' records count: #{Review.count}"
end

What I have in return shows that the books were inserted but not the associated reviews:

'Book' records count: 10
Last 'Book' reviews count: 0
'Review' records count: 0

I don't know if my way of inserting associated models in database is wrong or if the multi-level feature has an issue, I thought it was the best place to warn about this behaviour has @sferik 's PR #230 (thank you for that btw) was supposed to fix it.

This test was run on Ubuntu 15.10 with:

  • rails v4.2.5.1
  • activerecord v4.2.5.1
  • sqlite3 v1.3.11
  • activerecord-import from Github (at master@df866a4)

This post is a duplicate from my comment on #243.

Thank you.

@JHFirestarter
Copy link

@rhannequin ...see @allenwlee's findings in issue #231 (short answer is to add autosave: true and inverse_of in your models).

One problem I've found with this, in a common use case (dropped 2nd problem as it should be its own issue):

If there's also a model that has a has_many .. through association...such as book has_many :reviews and has_many :critics, through: :reviews, critic has_many :reviews and has_many :books, through: :reviews, and finally review belongs_to :book and belongs_to :critic. In this case, what @allenwlee shows will import either books and reviews or critics and reviews (with associations)...but not all 3. The rails docs say inverse_of "[does] not work with :through associations"...so I wonder if that's the core problem with the strategy.

@JHFirestarter
Copy link

also, make sure to use the latest version (0.11.0 was throwing off select errors)

@rhannequin
Copy link
Author

@JHFirestarter Thank you for your answer.

Unfortunately I'm facing the same results.

Using the same example project from the wiki, I updated the gem version from Github (now 0.12.0 from df866a4) and changed my Book model to:

class Book < ActiveRecord::Base
  has_many :reviews, inverse_of: :book, autosave: true
end

The task return is still the same, even if I added inverse_of and autosave:

'Book' records count: 10
Last 'Book' reviews count: 0
'Review' records count: 0

@JHFirestarter
Copy link

@rhannequin do you also have inverse_of and autosave: true in your Reviews model? Remember to use plural form in the Reviews model, i.e. inverse_of: :reviews

@rhannequin
Copy link
Author

@JHFirestarter I did not but just did and unfortunately it's still the same. My rake task hasn't changed and my only two models look like this:

class Book < ActiveRecord::Base
  has_many :reviews, inverse_of: :book, autosave: true
end

class Review < ActiveRecord::Base
  belongs_to :book, inverse_of: :reviews, autosave: true
end

Thank you again for your help.

@JHFirestarter
Copy link

Just making sure belongs_to: syntax above is bc you're copying from models with different actual names.

@rhannequin
Copy link
Author

Yes sorry, just updated my previous comment, the syntax is supposed to be correct.

@JHFirestarter
Copy link

@rhannequin np. only difference I can tell between what you and I have is (a) the db (I'm sure postgresql) and (b) potentially how we generated associations (i.e. I generated a review migration containing the line t.belongs_to :book, index: true) which adds to schema.rb both:
t.integer "book_id" and
add_index "reviews", ["book_id"], name: "index_reviews_on_book_id", using: :btree

Models contain:
Review model with belongs_to :book, inverse_of: :reviews, foreign_key: "book_id", autosave: true
Book model with has_many :reviews, inverse_of: :book, autosave: true

I'm out of (limited) ideas after that!

@rhannequin
Copy link
Author

@JHFirestarter My project will be on PostgreSQL but I'm using Sqlite3 for this example as I wanted to only test this multi-level feature of the gem.

My schema has been generated from the migrations mentionned above in my first comment, and looks like this:

ActiveRecord::Schema.define(version: 20160302142135) do

  create_table "books", force: :cascade do |t|
    t.string   "title"
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
  end

  create_table "reviews", force: :cascade do |t|
    t.string   "name"
    t.integer  "book_id"
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
  end

  add_index "reviews", ["book_id"], name: "index_reviews_on_book_id"

end

I should mention that I tested the association in the console, it works when I create a book with some reviews, the records are properly saved.

Thank you again.

@jkowens
Copy link
Collaborator

jkowens commented Mar 4, 2016

@rhannequin the :recursive option currently only works for Postgres. The documentation should be updated to make that clear. As of version 0.12.0 inverse_of and autosave are no longer required.

@rhannequin
Copy link
Author

@jkowens Thank you, that makes sense now. Is there any work in progress to cover Sqlite3 as it may be used on test and development environments? Or maybe is there a feature inside this gem that makes it impossible to work for Sqlite?

@jkowens
Copy link
Collaborator

jkowens commented Mar 4, 2016

I believe this feature could possibly be implemented for sqlite3, but I don't know if there is currently any work in progress to make that happen.

@yuri-karpovich
Copy link

@jkowens and what about mysql2 adapter? Looks like :recursive option doesn't work for it

@jkowens
Copy link
Collaborator

jkowens commented Mar 18, 2016

@yuri-karpovich you are correct this does not work for mysql2 either.

@zdennis
Copy link
Owner

zdennis commented Apr 1, 2016

If am reading this thread right the desired outcome is that the recursive options works for sqlite and mysql? Is that correct @yuri-karpovich and @jkowens?

@jkowens
Copy link
Collaborator

jkowens commented Apr 1, 2016

@zdennis that is how I understand it. In #104 @robmathews provided suggestions on how to possibly implement this functionality for SQLite and MySQL.

This was referenced May 31, 2016
@jkowens
Copy link
Collaborator

jkowens commented Jun 23, 2016

@rhannequin with the 0.14.0 release sqlite3 now supports recursive import.

@rhannequin
Copy link
Author

@jkowens Thank you, I should be able to test it soon.

@jkowens jkowens removed the sqlite label Oct 4, 2016
@jkowens jkowens closed this as completed Oct 25, 2016
@yuri-karpovich
Copy link

@jkowens and what about mysql?

@jkowens
Copy link
Collaborator

jkowens commented Oct 26, 2016

@yuri-karpovich sorry I should have added a comment to this before closing. Unfortunately there currently does not seem to be a good way to implement this feature for MySQL. See PR #279 for some more details. Unless MySQL implements an option to return inserted/updated IDs like PostgreSQL, we will not be able to make this work in a reliable way.

@yuri-karpovich
Copy link

@jkowens thank you for information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants