Rails Ruby on Rails → 2.3.6

Comments and changes to this ticket

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  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.
  

• You flagged this item as spam.
  

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

  Ok, here is the fix including tests from graeme

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

• You flagged this item as spam.
  

  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

  • association_preload.rb.diff 272 Bytes

• You flagged this item as spam.
  

  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?

• You flagged this item as spam.
  

  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])

• You flagged this item as spam.
  

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

  apparently my issue was related to http://rails.lighthouseapp.com/p...

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  gbp February 18th, 2009 @ 06:53 PM

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

• You flagged this item as spam.
  

  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?

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  gbp March 17th, 2009 @ 11:38 AM

  Updated patch for rails 2.3.2

  • has_one_foreign_and_primary_keys_bug.diff 5.1 KB
• 

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

  • Assigned user set to “Eloy Duran”
• 

  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 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 September 11th, 2009 @ 11:04 PM

  • Milestone changed from “2.3.4” to “2.3.6”

  [milestone:id#50064 bulk edit command]

• 

  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 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...

• You flagged this item as spam.
  

  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

• You flagged this item as spam.
  

  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”