Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rely on through table name in has_many fixtures #17426

Merged
merged 1 commit into from
Feb 25, 2015
Merged

Rely on through table name in has_many fixtures #17426

merged 1 commit into from
Feb 25, 2015

Conversation

jpcody
Copy link
Contributor

@jpcody jpcody commented Oct 29, 2014

The fixtures code always builds join table names via the association's join_table method, which will use derive_join_table to build the table name via convention. But in the event a join table was created with a different table name, this will fall down. Using the through_association#table_name makes an affordance for both of these cases.

I think the existing code works from for the ReflectionProxy but should be overridden for the HasManyThroughProxy to handle if the table name is either overridden in the model or created with a different table name.

Rather than using the association's join_table method, which
constructs a table name from conventions, this should rely on the
through reflection's table_name to be resilient to tables that were
not automatically named.
@wyaeld
Copy link
Contributor

wyaeld commented Feb 24, 2015

Some more background, we ran into this recently and came up with the same solution before finding @jpcody PR. We've ran the same code, and it will load has_many through fixtures correctly, as the HABTM does.

@tenderlove you had 2 commits relating to this 59ba85f & a9da99e
the problem is that the 2nd in the second one the HasManyThroughProxy doesn't implement join_table and therefore just inherits the ReflectionProxy implementation, which is based on Rails naming convention, and ignores the :through option

The test case with Parrot Treasure ParrotTreasure only tests HABTM.

If it was

class Parrot 
has_many :secrets
has_many :treasures, through: secrets

then you avoid the assumption on naming conventions

ping @senny as well just in case.

@tenderlove
Copy link
Member

Oops! Makes sense.

tenderlove added a commit that referenced this pull request Feb 25, 2015
Rely on through table name in has_many fixtures
@tenderlove tenderlove merged commit f4bee7e into rails:master Feb 25, 2015
tenderlove added a commit that referenced this pull request Feb 25, 2015
Rely on through table name in has_many fixtures
tenderlove added a commit that referenced this pull request Feb 25, 2015
Rely on through table name in has_many fixtures
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants