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

Avoid private ActiveRecord API when lazily registering container attributes #214

Merged
merged 2 commits into from
Nov 2, 2023

Conversation

jrochkind
Copy link
Owner

We were using a attributes_to_define_after_schema_loads method which was private API, and which breaks in current Rails main branch post 7.1 release.

We switch to only using standard attribute API, and keeping track ourselves internally if this is the "first time" a container attrib is encountered, so if it needs an attribute registration. We don't want to multiply register the same attribute many times becaues although it would work, it's just messy and extra objects and calls for rails.

This is working -- we had pretty good test coverage, which helped us realize we needed to track specific sub-class too, and tests now all pass.

In the long-run, we might like a different public API that doesn't require us to implicitly know when first time container attrib is encountered. Like requires you to explicitly group things by container attribute. Or... this works and is fine? Could be!

This is one part of #212 although other failures are revealed once we fix this one.

…ibutes

We were using a `attributes_to_define_after_schema_loads` method which was private API, and which breaks in current Rails main branch post 7.1 release.

We switch to only using standard attribute API, and keeping track ourselves internally if this is the "first time" a container attrib is encountered, so if it needs an attribute registration. We don't want to multiply register the same attribute many times becaues although it would work, it's just messy and extra objects and calls for rails.

This is working -- we had pretty good test coverage, which helped us realize we needed to track specific sub-class too, and tests now all pass.

In the long-run, we might like a different public API that doesn't require us to implicitly know when first time container attrib is encountered. Like requires you to explicitly group things by container attribute. Or... this works and is fine? Could be!

This is one part of #212 although other failures are revealed once we fix this one.
@jrochkind jrochkind merged commit b2cd414 into master Nov 2, 2023
10 checks passed
@jrochkind jrochkind deleted the avoid_ar_private_attribute_registration branch November 2, 2023 18:00
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.

1 participant