Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Check joined table name by table_name #8630

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

route commented Dec 27, 2012

In my app I use STI and overridden table_name but habtm in it doesn't work properly, and I have to set options on it forcibly. I'll show you example:

module Namespace
  class Base < ActiveRecord::Base
    self.table_name = 'tablename'
    has_and_belongs_to_many :somethings
  end
end

class Namespace::Child < Namespace::Base
end

It gives me advantage I can keep all these stuff inside namespace directory and it will be auto loaded by AS and it's still STI. But HABTM relies on classes when it calculates table names, so for join table it chooses bases_somethings instead of tablename_somethings. Of course I can set these options manually and it will work. I left comment in this PR and it has small refactoring proposed by @jonleighton in comment and actually this refactoring is the way to go if we want to use table_name.

Owner

pixeltrix commented Dec 27, 2012

This is against 3-2-stable which wouldn't be acceptable since it may break existing apps. I did some similar work on master in 4649294 - I may not have covered everything so do please check it works as you expect.

Contributor

route commented Dec 27, 2012

@pixeltrix Yes, sorry I rushed in fixing it against my app and 3-2 and hadn't checked it against master. This doesn't have incompatible changes for now, couldn't it be acceptable anyway? If we can do some refactoring on 3-2 I'll check your changes and merge it with mine if they are back compatible. I meant just to pull that logic into Reflection. I'll check your commit against master.

Owner

pixeltrix commented Dec 28, 2012

@route doesn't your commit use the table name rather than the class name? That's what your original message suggests.

Contributor

route commented Dec 28, 2012

For now I just moved that logic to Reflection without any changes to ask opinions. So if we cannot use table_name there because it's not back compatible and we're not going to apply refactoring I tend to close this and keep checking master branch ;)

Owner

pixeltrix commented Dec 28, 2012

OK - closing

@pixeltrix pixeltrix closed this Dec 28, 2012

@route route referenced this pull request Jan 10, 2013

Closed

Added foreign_key option #8867

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment