-
Notifications
You must be signed in to change notification settings - Fork 694
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
Reflect Rails' serializer changes #664
Conversation
As a result of [Rails PR#47463](rails/rails#47463), serialization is now type-checked, and so the approach of dynamically deciding on `#load` and `#dump` no longer works. This commit adds a configuration setting to choose json encoding by default (replaces the existing approach where the coder was dynamaically determined by database column type)
This is a significant upcoming change from Rails. @danielmorrison @tiagocassio Any chance this could be merged in? (Sorry for the random ping, I just grabbed the two most recent PR mergers I saw; since this has been open since February I'm not sure how actively this repo is watched). If this is PR indeed a breaking change, there's also another Rails deprecation notice nearby:
The code here suggests this can just be removed, too: Lines 23 to 25 in 379aeed
|
Is this gem still maintained? |
Yeah, I'm still around, but not as attentive as I should be. I'll make some time to check this out. |
No worries—happy to help clean up / merge the present PRs, or do anything else that would be contributive. 👍 |
👋 @danielmorrison Rails 7.1 is coming out soon, so the deprecation here will be a failure in Rails 7.2. Note that the PR here won't get rid of all the deprecation warnings; there's another line I pointed to earlier. I don't think there's a PR open for that removal but happy to open one up. |
BTW, I came across this block of code which would achieve the same future forward change without breaking any semver compatibility: https://github.com/gauravtiwari/noticed/blob/d14878a95602e56967f3b558b402f7f863608d91/lib/noticed/model.rb#L16C1-L19 |
Looks like the newer #686 resolved this. |
Fixed via #686 |
As a result of Rails PR#47463, serialization is now type-checked, and so the approach of dynamically deciding when calling
#load
and#dump
no longer works.This means that you'll get the following errors/deprecation warnings when using Edge Rails with Audited:
And:
(The first is just a syntax thing to be fair)
I don't know the best way to fix it to be honest - determining the column type would require connecting to the database on load, so that's sub-optimal. It might be possible to implement a
Coder
class that dynamically switches and passes the type checking?This is the commit I'm using, which adds a configuration setting to choose json encoding by default (replaces the existing approach). This seems like something worth considering as it seems Rails has moved away from YAML for good reasons too.
I've sent the PR in case it's useful, but of course it's a breaking change and I think there might be a better approach - figured I'd start the discussion anyway 😄
Thank you!