-
Notifications
You must be signed in to change notification settings - Fork 35
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
Rails 7 support #55
Comments
@olegantonyan hey, do you know what needs to be changed? Im in a pickle because of this gem not being updated to 7. Id like to fork and make the changes myself but i don't understand the inner workings of active record. |
I use own fork https://github.com/olegantonyan/polymorphic_integer_type But there is a problem with has_one association. Don't remember the details exactly, but I had to work around it. So if you end up going this route - test it thoroughly |
Rails 7 has been supported since v3.2.0. However Rails 7.1 is not supported, the gemspec explicitly restricts the package to depend on Rails 7.0 and below. @wendy-clio are there any plans to support Rails 7.1 ? What's the main issue with supporting Rails 7.1? Rails 7.2 will be released soon too |
Although I haven't dug into it properly, I doubt there are many blockers to upgrading to Rails 7.1. The main problem is that the gem doesn't have an actual owner. Although I am happy to try upgrading the gem, I can only do it in my spare time. Unfortunately, that means I can't provide a solid timeline. Another point worth noting: AFAIK, this gem's primary usage is in our manage codebase, which is still on Rails 6.1. We won't gain much organizational benefit immediately. |
@wendy-clio thanks for the update! We might work on the support ourselves once we get close to upgrading from Rails 7.0 to 7.1, if it's not done by then. But I cannot yet tell regarding when we can get there. If we prepare a PR for supporting Rails 7.1, can we expect it to be reviewed and accepted? Is there anything special we may need to know to work on that? |
We may explore support of 7.2 as a part of that too |
Yes, for sure. I can help and most staff should be happy to review it.
Not I can think of now. |
Hello @wendy-clio ! I made some research on Rails 7.1 support with Here's what I've found. There's one test that fails once Rails is set to 7.1 version. Here's the test that fails:
I've tracked down the change in Rails that is causing the failure. The change is this line: Previously what was happening is that in Rails during creation of the record with association assignment was used:
Now it has been replaced with
The result @wendy-clio so I successfully tracked down the change that causes the tests to fail now. However I'm still unsure on what we can do on the gem's side and what would we need to update in order to fix the gem and make it compatible with Rails 7.1. @wendy-clio is there any suggestions you might provide on the potential fix? I assume you know more about about how the gem integrates with Rails. |
Sorry for the delay. I think the main problem is somehow rails changes its behaviour of not reloading the activeRecord after polymorphic_integer_type/lib/polymorphic_integer_type/polymorphic_foreign_association_extension.rb Line 7 in c9ad377
or to keep the same pattern with rails update, just add a reload on the failing test which is the typical behaviour of needing to reload the owner object after the association has create directly against db. |
Is it already in progress? I had to change few things to make it work (kind of, not perfectly). But maybe a proper support is around the corner.
Also, a bit related, is there a chance for this PR to be merged? #53
The text was updated successfully, but these errors were encountered: