This project is archived and is in readonly mode.

#1756 ✓resolved
gbp

has_one with :foreign_key & :primary_key bug

Reported by gbp | January 14th, 2009 @ 02:21 AM | in 2.3.6

Similar to #1633

has_one associations with :foreign_key and :primary_key options don't work as expected.

After saving the associations are being lost. The :foreign_key is being assigns to the id of the owner instead of using the provided :primary_key option.

Fix and unit test attached.

Comments and changes to this ticket

  • Brian Johnson

    Brian Johnson February 2nd, 2009 @ 06:19 AM

    I just ran into this same bug and this patch is incomplete. I will generate work on a test case and patch.

  • Brian Johnson

    Brian Johnson February 2nd, 2009 @ 07:39 AM

    ok, should be

    associations.rb

    @@@ def has_one(association_id, options = {})

        if options[:through]
          reflection = create_has_one_through_reflection(association_id, options)
          association_accessor_methods(reflection, ActiveRecord::Associations::HasOneThroughAssociation)
        else
          reflection = create_has_one_reflection(association_id, options)
    
          ivar = "@#{reflection.name}"
    
          method_name = "has_one_after_save_for_#{reflection.name}".to_sym
          define_method(method_name) do
            association = instance_variable_get(ivar) if instance_variable_defined?(ivar)
    
            primary_key = reflection.options[:primary_key] || :id
            if !association.nil? && (new_record? || association.new_record? || association[reflection.primary_key_name] != send(primary_key))
              association[reflection.primary_key_name] = send(primary_key)
              association.save(true)
            end
          end
          after_save method_name
    
          add_single_associated_validation_callbacks(reflection.name) if options[:validate] == true
          association_accessor_methods(reflection, ActiveRecord::Associations::HasOneAssociation)
          association_constructor_method(:build,  reflection, ActiveRecord::Associations::HasOneAssociation)
          association_constructor_method(:create, reflection, ActiveRecord::Associations::HasOneAssociation)
    
          configure_dependency_for_has_one(reflection)
        end
      end
    
    
    
    
    has_one_association.rb
    
    @@@ def new_record(replace_existing)
            # Make sure we load the target first, if we plan on replacing the existing
            # instance. Otherwise, if the target has not previously been loaded
            # elsewhere, the instance we create will get orphaned.
            load_target if replace_existing
            record = @reflection.klass.send(:with_scope, :create => construct_scope[:create]) do
              yield @reflection
            end
    
            if replace_existing
              replace(record, true) 
            else
              puts "test"
              record[@reflection.primary_key_name] = @reflection.options.has_key?(:primary_key) ? @owner.send(@reflection.options[:primary_key]) : @owner.id unless @owner.new_record?
              self.target = record
            end
    
            record
          end
    

    association_proxy.rb

    @@@ def set_belongs_to_association_for(record)

        if @reflection.options[:as]
          record["#{@reflection.options[:as]}_id"]   = @owner.id unless @owner.new_record?
          record["#{@reflection.options[:as]}_type"] = @owner.class.base_class.name.to_s
        else
          record[@reflection.primary_key_name] = @reflection.options.has_key?(:primary_key) ? @owner.send(@reflection.options[:primary_key]) : @owner.id unless @owner.new_record?
        end
      end
    
    
    
    
    Sorry I don't have a real patch, it's late and I'm tired.
    
  • Brian Johnson

    Brian Johnson February 2nd, 2009 @ 08:39 PM

    Ok, here is the fix including tests from graeme

    http://github.com/johnsbrn/rails...

  • Thomas Shelton

    Thomas Shelton February 17th, 2009 @ 11:38 PM

    I'm a complete noob here so forgive me if I missed something. I believe there is still a bug with respect to this fix. Although the association works when referenced directly from an object, when the associations are loaded through an :include, they still reference the original primary key.

    I changed association_preload.rb as described in the attached diff.

    Is this something that can be added to the proposed fix or should I do something further?

    Thanks, Thomas

  • Brian Johnson

    Brian Johnson February 17th, 2009 @ 11:45 PM

    If you can make a test case for it I will add it to my git repo. Not sure what the best way to get this back into the main repo though, maybe I should do a pull request?

  • Brian Johnson

    Brian Johnson February 18th, 2009 @ 04:24 PM

    just FYI - looks like this is already changed in edge, but the association isn't preloading for me. I'm working on a test case for it.

    id_to_record_map, ids = construct_id_map(records, reflection.options[:primary_key])

  • Brian Johnson
  • Brian Johnson

    Brian Johnson February 18th, 2009 @ 06:39 PM

    Ok, everything is good on edge rails with include, but I added a test case to the git repo anyway.

  • gbp

    gbp February 18th, 2009 @ 06:53 PM

    For the record: original problem still exists. The tests in my patch still fail on edge rails.

  • Brian Johnson

    Brian Johnson February 19th, 2009 @ 06:07 PM

    That's really strange because id_to_record_map, ids = construct_id_map(records, reflection.options[:primary_key]) is functionally equivalent to your patch. You didn't include the failing test, can you provide it?

  • gbp

    gbp February 25th, 2009 @ 12:24 AM

    My problem resides in associations.rb (currently Line 906) - not association_preload.rb. On edge rails the test provided in my original patch do fail without the patch to associations.rb being applied.

  • Brian Johnson

    Brian Johnson February 25th, 2009 @ 04:35 AM

    Graeme, your tests pass on my fork. Thomas, I'd like to include your issue as well, but I need a failing test first.

  • gbp
  • Eloy Duran

    Eloy Duran March 17th, 2009 @ 06:06 PM

    • Assigned user set to “Eloy Duran”
  • Michael Koziarski

    Michael Koziarski June 9th, 2009 @ 09:35 AM

    • Milestone changed from 2.x to 2.3.4

    Can you take a look at this eloy?

  • Eloy Duran

    Eloy Duran July 12th, 2009 @ 12:38 PM

    • State changed from “new” to “verified”

    I have verified and applied this patch on my branch of 2-3-stable, which Micheal will look at and merge in before 2.3.4.

    http://github.com/alloy/rails/commit/e5de99082a7d151a70e8d48b96471f...

    Thanks!

  • Jeremy Kemper

    Jeremy Kemper September 11th, 2009 @ 11:04 PM

    • Milestone changed from 2.3.4 to 2.3.6

    [milestone:id#50064 bulk edit command]

  • Repository

    Repository September 13th, 2009 @ 02:33 AM

    • State changed from “verified” to “resolved”

    (from [a070873771ebede9097735f07fc40720dce89c46]) Fix has_one with foreign_key and primary_key association bug which caused the associated object being lost when saving the owner. [#1756 state:resolved]

    Mixed in a bit from patch by ransom-briggs. [#2813 state:resolved]

    Signed-off-by: Eloy Duran eloy.de.enige@gmail.com
    http://github.com/rails/rails/commit/a070873771ebede9097735f07fc407...

  • Repository

    Repository September 13th, 2009 @ 02:33 AM

    (from [c01be9de322ba846923340e41e69821d01541610]) Fix has_one with foreign_key and primary_key association bug which caused the associated object being lost when saving the owner. [#1756 state:resolved]

    Mixed in a bit from patch by ransom-briggs. [#2813 state:resolved]

    Signed-off-by: Eloy Duran eloy.de.enige@gmail.com
    http://github.com/rails/rails/commit/c01be9de322ba846923340e41e6982...

  • tribalvibes

    tribalvibes September 17th, 2010 @ 09:12 AM

    • Tag changed from activerecord, foreign_key, patch, primary_key to 2.3.10, 2.3.8, activerecord, foreign_key, has_many, patch, primary_key
    • Importance changed from “” to “High”

    Similar to #1633 which is still a bug in 2.3.8
    has_many association.create doesn't set the new record fk correctly when the association pk is designated with :primary_key

  • Andrea Campi

    Andrea Campi October 16th, 2010 @ 11:35 PM

    • Tag changed from 2.3.10, 2.3.8, activerecord, foreign_key, has_many, patch, primary_key to 2-3-stable, activerecord, foreign_key, has_many, patch, primary_key

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Referenced by

Pages