Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

build assocation does not set reference to parent #1920

Closed
gucki opened this Issue · 8 comments

4 participants

@gucki
Customer
   has_many :offers
end

Offer
  belongs_to :customer
end

c = Customer.new
c.offers.build

c.offers.first.customer => nil (should be same as c!)

@jonleighton
Collaborator

You are dealing with two new records and are not using :inverse_of. Therefore, there is no way for us to know that customer is the inverse of offers. Use :inverse_of and it should work.

@gucki

Hm I though I also tried with inverse_of, but I'll check again to make sure.

I'm not really sure why rails can't guess it - isn't it obvious in most cases, like this one?

@gucki

Ok, it's working fine with inverse_of. I must have made some mistake.

But I dont get why the inverse cannot be determined automatically. All cases I saw so far look trivial and I'd be happy to provide a patch to make the setting of inverse_of automatic.

http://mediumexposure.com/better-memory-associations-inverseof/
http://apidock.com/rails/ActiveRecord/Associations/ClassMethods (Bi-directional associations)

Or can you provide me with some examples where it'd not be possible automatically but manually? I really cannot find one.

@jonleighton
Collaborator

I think the main reason not to guess it based on association / foreign key names is that this 'magic' could lead to unexpected behaviour to users who are not familiar with that is going on. If you setup one thing in a model you don't necessarily expect it to affect how another model behaves.

I am curious about what @tenderlove, @josevalim and @pixeltrix have to say on this.

@gucki

I think right now the behavior is much more unexpected. When I build an association through another model I simply expect that the reference to the parent is valid and not nil. It'd give a nice performance improvements for free in many cases too.

I am also very curious about their answers :)

@pascalbetz

I'd expect the object to be present as well... not sure in what situations this would go wrong? Interested in it as well.

@pixeltrix
Owner

If you still have access to the LH tracker then the original ticket discusses this issue, however basically the reasoning was to make it explicit so that it didn't change existing associations and wasn't 'magical'. I think this reasoning is still sound but maybe it could be flipped if/when IdentityMap is enabled by default. The original patch was based upon a plugin called parental_control which did automatically guess the association names. One thing we would need to be able to do is disable the automatic lookup in the cases where there's a matching association that isn't the correct one. I'm guessing this could be done by passing :inverse_of => false or :inverse_of => nil.

One issue that may be a problem is where you have multiple matching associations - if you take the slug model from the friendly_id gem it has both a has_one :slug and a has_many :slugs: e.g:

class Product < ActiveRecord::Base
  has_many :slugs, :order => 'id DESC', :as => :sluggable, :dependent => :destroy
  has_one :slug, :order => 'id DESC', :as => :sluggable, :dependent => :nullify
end

class Slug < ActiveRecord::Base
  belongs_to :sluggable, :polymorphic => true
  # Is this inverse_of :slug, :slugs or both?
end

However a lot of the possibly tricky situations aren't currently handled by :inverse_of (like belongs_to mapping to has_many) anyway so it's possible they won't be an issue. I think this is the current list of invertible associations:

belongs_to -> has_one
belongs_to_polymorphic -> has_one
has_one -> belongs_to
has_one -> belongs_to_polymorphic
has_one_through -> belongs_to
has_one_through -> belongs_to_polymorphic
has_many -> belongs_to
has_many -> belongs_to_polymorphic

It's certainly it's worth investigating to see if it can be done for the above mappings.

@jonleighton
Collaborator

If anyone wants to work on this, I suggest you create a plugin and see what issues you come up against (and document them!) After that, we can consider inclusion in Rails itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.