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

fixed joins association #22

Merged
merged 1 commit into from
Apr 6, 2016
Merged

fixed joins association #22

merged 1 commit into from
Apr 6, 2016

Conversation

stasl
Copy link
Contributor

@stasl stasl commented Mar 30, 2016

No description provided.

@jonkessler
Copy link
Contributor

Thanks for the contribution! Would you mind adding a test for this change?

@stasl stasl force-pushed the master branch 4 times, most recently from 836ca16 to 0858602 Compare April 5, 2016 21:43
@stasl stasl force-pushed the master branch 2 times, most recently from 1167b19 to 65d7690 Compare April 5, 2016 22:01
@stasl
Copy link
Contributor Author

stasl commented Apr 6, 2016

@jonkessler I have added spec and few more fixes, please check

else
klass.send(:build_default_scope, ActiveRecord::Relation.create(klass, table))
end
scope_chain_items.concat [default_scope].compact
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change in Rails 4.1.x somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, method signature was change in 4.1.x, then added default parameter.
specs failed for that versions, this is how I discovered

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Thanks!

@jonkessler jonkessler merged commit 2c5fb00 into appfolio:master Apr 6, 2016
@jonkessler
Copy link
Contributor

Thanks!

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.

2 participants