Comments on #9575 requested please.

I've just created a new ticket with a patch that adds some extra searching to compute_type in AR. It starts with the existing behaviour and then walks up the model namespace. It also tries a name prefixed with current name so that it can catch models prefix with their parent's name. e.g. This model:

   class Shop::Product < ActiveRecord::Base
     has_many :items
   end

   class Shop::ProductItem < ActiveRecord::Base
   end

would give the follow candidate names:

   ::Shop::Item
   ::Item
   ::Shop::Product::Item
   ::Shop::ProductItem
   ::Shop::Item
   ::ShopItem

It passes all the existing tests but one of the changes it makes is to prefix the existing names that are checked with '::' to limit the scope of the check, so is there any chance that any of you out there with complicated models can check that it doesn't break your app? Also AFAICS the association reflection caches this result so performance impact should negligible - is this assumption correct?

Any other comments would be appreciated too.

Andrew White

I'm not really sure why you want it to find ProductItem in addition to
Item? Couldnt you just use has_many :product_items?

You can but you end up with accessing stuff like @products.product_items which is a bit redundant. If you name the class Product::Item then referring to the nested class from somewhere else requires the full name and the reciprocal relationship requires a leading '::' e.g:

   class Product < ActiveRecord::Base
     has_many :items
   end

   class Product::Item < ActiveRecord::Base
     belongs_to :product, :class_name => '::Product'
   end

   class Basket < ActiveRecord::Base
     has_many :items
   end

   class Basket::Item < ActiveRecord::Base
     belongs_to :basket
     belongs_to :product_item, :class_name => '::Product::Item'
   end

I've no doubt that my patch isn't perfect but I don't think the existing compute_type is adequate either.

Andrew

You can but you end up with accessing stuff like
@products.product_items which is a bit redundant. If you name the
class Product::Item then referring to the nested class from somewhere
else requires the full name and the reciprocal relationship requires
a leading '::' e.g:

If there are issues with using the association macros between classes
in modules, I think we could look into fixing them. But I definitely
don't think that the item -> ProductItem case is justified.

If you could write up some test cases which currently fail, then we
can figure out where to go from here.

I went a bit further a came up with a set of tests that show all the issues with nested models that I can find. I've attached them to the same ticket.

Andrew White