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

fix(associations): redefine build_record for CollectionAssociation #26

Conversation

aergonaut
Copy link
Contributor

In Rails 6.0.4, CollectionAssociation defines its own implementation of build_record which overrides the one inherited from the Association base class. This breaks protected_attributes_continued's redefinition because now CollectionAssociation is no longer using the implementation from the super class.

I fixed this by checking if CollectionAssociation defines its own build_record by using private_instance_methods(false). If it does, I undef'd it, and then added a new implementation that accepts the extra options argument. The implementation is just the same as the one from main.

See this commit for the addition of build_record to CollectionAssociation: rails/rails@1192867

Only appears in the 6-0-stable branch, first released in the v6.0.4 tag.

Does not appear in the 6-1-stable or main branches. I assume the issue this was fixing is not present in those branches, but I'm not sure. However, in 6.1.x, private_instance_methods(false).include?(:build_record) returns false, so I think the conditional I used to target this patch should only hit 6.0.4.

@westonganger
Copy link
Owner

westonganger commented Jun 21, 2021

Thanks for this. Yes it would be better if you can use a Rails version conditional rather than the if self.private_instance_methods(false).include?(:build_record)

In Rails 6.0.4, CollectionAssociation defines its own implementation of
build_record which overrides the on inherited from the Association base
class. This breaks protected_attributes_continued's redefinition because
now CollectionAssociation is no longer using the implementation from the
super class.

I fixed this by adding a new override for build_record in
CollectionAssociation. The override is only applied for Rails >= 6.0.4 <
6.1. The implementation is just the same as the one from main.
@aergonaut aergonaut force-pushed the fix/rails-6-0-4-collection-association-build-record branch from 573ccc7 to 9287c9a Compare June 21, 2021 22:56
@aergonaut
Copy link
Contributor Author

@westonganger Got it. Updated like you suggested. Now it checks specifically for Rails >= 6.0.4 < 6.1.

@westonganger westonganger merged commit d9c31a3 into westonganger:master Jun 22, 2021
@aergonaut aergonaut deleted the fix/rails-6-0-4-collection-association-build-record branch June 23, 2021 00:48
@aergonaut aergonaut mentioned this pull request Jul 13, 2021
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

2 participants