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

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

Closed
jacob-carlborg-apoex opened this issue Aug 21, 2024 · 3 comments
Labels

Comments

@jacob-carlborg-apoex
Copy link
Contributor

If I have a class structure like this:

class Foo < ActiveRecord::Base
  has_many :items, class_name: "FooItem"
end

class FooItem < ActiveRecord::Base
end

And try to import using recursive_on_duplicate_key_update, like this:

Foo.import(records,
  on_duplicate_key_update: {columns: :all},
  recursive_on_duplicate_key_update: {items: {columns: :all}})

Then activerecord-import won't do an upsert for the association, only for the main records.

The issue is that options[:recursive_on_duplicate_key_update][table_name] returns nil here:

on_duplicate_key_update: options[:recursive_on_duplicate_key_update][table_name]

The code is looking for a key matching the table name of the association instead of the name of the association itself. The documentation says: "The hash key is the association name".

@jkowens jkowens added the bug label Aug 23, 2024
@jkowens
Copy link
Collaborator

jkowens commented Aug 23, 2024

Hi @jacob-carlborg-apoex , thanks for reporting this issue. I assume if you do the following it will work?

Foo.import(records,
  on_duplicate_key_update: {columns: :all},
  recursive_on_duplicate_key_update: {foo_items: {columns: :all}})

If you are able to provide a PR to correct this issue that would be appreciated!

@jacob-carlborg-apoex
Copy link
Contributor Author

Yes, with foo_items it works.

If you are able to provide a PR to correct this issue that would be appreciated!

What's the policy on breaking changes? Changing this would be a breaking change. Should the code try to look for both names or should this be a breaking change?

@jkowens
Copy link
Collaborator

jkowens commented Aug 23, 2024

Good point, we can go ahead and make a breaking change. We can release in in v2.0 since we loosely follow semver 😅

@jkowens jkowens closed this as completed in 2405beb Oct 4, 2024
jkowens added a commit that referenced this issue Oct 4, 2024
…ation-843

Fix #843: recursive_on_duplicate_key_update doesn't work with non-standard association name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants