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

Allow constructors for has_one :through #40007

Merged

Conversation

@perezperret
Copy link
Contributor

@perezperret perezperret commented Aug 7, 2020

Summary

I couldn't find any reference to this in the guides or docs, but association constructors for has_one :through associations appear to be disabled. I simply enabled them and used them in the tests and nothing broke, so I'm trying to understand why they should be disabled. I would appreciate some guidance for figuring this out. Also, if there is a good reason for this, or this change is deemed unimportant or unnecessarily risky, I think the docs should be updated to reflect that build_association and create_association are not available for has_one :through (I'd be happy to make those changes instead if that's the case).

@rails-bot
Copy link

@rails-bot rails-bot bot commented Nov 5, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Nov 5, 2020
@rafaelfranca
Copy link
Member

@rafaelfranca rafaelfranca commented Nov 5, 2020

This is disabled since the feature was introduced 273b21f. I think we should try to enable it.

@rails-bot rails-bot bot removed the stale label Nov 5, 2020
@perezperret
Copy link
Contributor Author

@perezperret perezperret commented Nov 5, 2020

@rafaelfranca I can add a few more tests based on the other associations. Any other suggestions on things to look out for? Any ideas as to why it wasn't originally included?

@rafaelfranca
Copy link
Member

@rafaelfranca rafaelfranca commented Nov 6, 2020

That was before my time. I'd say, let's try to improve the test coverage and add an changelog entry. We can release in the 6.2 release.

@perezperret perezperret force-pushed the perezperret:spp-has-one-through-constructable branch from ce9e4d0 to b220054 Nov 9, 2020
@perezperret perezperret force-pushed the perezperret:spp-has-one-through-constructable branch from b220054 to ac61973 Nov 9, 2020
@perezperret
Copy link
Contributor Author

@perezperret perezperret commented Nov 9, 2020

Great! I added some simple tests and a changelog entry.

@rafaelfranca rafaelfranca merged commit 8ead8b3 into rails:master Nov 9, 2020
2 checks passed
2 checks passed
@github-actions
build
Details
@buildkite
buildkite/rails Build #72698 passed (10 minutes, 40 seconds)
Details
@perezperret perezperret deleted the perezperret:spp-has-one-through-constructable branch Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants