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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

insert with existing association does not seem to work #804

Open
jwoodrow opened this issue Jun 13, 2023 · 0 comments
Open

insert with existing association does not seem to work #804

jwoodrow opened this issue Jun 13, 2023 · 0 comments

Comments

@jwoodrow
Copy link

jwoodrow commented Jun 13, 2023

Hi 馃憢

I'm trying to insert multiple new records with a has_many association but it seems like it's not working correctly in this case

class Vehicle < ActiveRecord::Base
  has_many :drivers
end

class Driver < ActiveRecord::Base
  belongs_to :vehicle
end

driver = Driver.create
vehicle = Vehicle.new(drivers: [driver])

Vehicle.import([vehicle], recursive: true, on_duplicate_key_update: :all)
driver.reload.vehicle_id #=> nil

when digging a bit I found that here although models.first.id is set correctly, models.first.association(:drivers).target.first.vehicle.id is nil like its a copy. so since vehicle_id, the foreign key is not set then changed? returns false

Shouldn't this foreign key assignment be done before, so if the only change is the belongs_to association it still detects a change ?

changing

def find_associated_objects_for_import(associated_objects_by_class, model)
associated_objects_by_class[model.class.name] ||= {}
return associated_objects_by_class unless model.id
association_reflections =
model.class.reflect_on_all_associations(:has_one) +
model.class.reflect_on_all_associations(:has_many)
association_reflections.each do |association_reflection|
associated_objects_by_class[model.class.name][association_reflection.name] ||= []
association = model.association(association_reflection.name)
association.loaded!
# Wrap target in an array if not already
association = Array(association.target)
changed_objects = association.select { |a| a.new_record? || a.changed? }
changed_objects.each do |child|
child.public_send("#{association_reflection.foreign_key}=", model.id)
# For polymorphic associations
association_name = if model.class.respond_to?(:polymorphic_name)
model.class.polymorphic_name
else
model.class.base_class
end
association_reflection.type.try do |type|
child.public_send("#{type}=", association_name)
end
end
associated_objects_by_class[model.class.name][association_reflection.name].concat changed_objects
end
associated_objects_by_class
end
like such seems to resolve my issues

def find_associated_objects_for_import(associated_objects_by_class, model)
  associated_objects_by_class[model.class.name] ||= {}
  return associated_objects_by_class unless model.id

  association_reflections =
    model.class.reflect_on_all_associations(:has_one) +
    model.class.reflect_on_all_associations(:has_many)
  association_reflections.each do |association_reflection|
    associated_objects_by_class[model.class.name][association_reflection.name] ||= []

    association = model.association(association_reflection.name)
    association.loaded!

    # Wrap target in an array if not already
    association = Array(association.target)

    # This line is added
    association.each { |child| child.public_send("#{association_reflection.foreign_key}=", model.id) }
    changed_objects = association.select { |a| a.new_record? || a.changed? }
    changed_objects.each do |child|
      # Removed the foreign_key assignment here
      # For polymorphic associations
      association_name = if model.class.respond_to?(:polymorphic_name)
        model.class.polymorphic_name
      else
        model.class.base_class
      end
      association_reflection.type.try do |type|
        child.public_send("#{type}=", association_name)
      end
    end
    associated_objects_by_class[model.class.name][association_reflection.name].concat changed_objects
  end
  associated_objects_by_class
end

Current workaround is also to add driver.send(:mutations_from_database).force_change('vehicle_id') right after assigning the new vehicle to the existing driver. The fact that mutations_from_database is a private method is kind of an indicator that this is not really what it's made for

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

No branches or pull requests

1 participant