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

Rails 5 is removing serialized_attributes without replacement #416

Closed
thetron opened this issue Sep 4, 2014 · 28 comments
Closed

Rails 5 is removing serialized_attributes without replacement #416

thetron opened this issue Sep 4, 2014 · 28 comments
Milestone

Comments

@thetron
Copy link

thetron commented Sep 4, 2014

Hey papertrailers,

We're currently building a Rails 4.2 app with paper_trail (love it, btw), and getting the following warning:

DEPRECATION WARNING: `serialized_attributes` is deprecated without replacement, and will
be removed in Rails 5.0.

Which I noticed paper_trail calls several times in has_paper_trail.rb.

I'm sure you're aware of it already - but just wanted to flag it. I did have a little poke around to see if there was an obvious solution - but I couldn't come up with one. Happy to put together a PR, if someone could offer some guidance.

@batter batter changed the title Rails 4.2 is deprecating serialized_attributes without replacement Rails 5 is deprecating serialized_attributes without replacement Sep 4, 2014
@JeanMertz
Copy link

@batter regarding your title change, if the warning @thetron pasted is accurate, then it is deprecated in 4.2 and will be removed in 5.0, instead of being deprecated in 5.0.

Just a heads up 👍

@batter batter changed the title Rails 5 is deprecating serialized_attributes without replacement Rails 5 is removing serialized_attributes without replacement Sep 5, 2014
@batter batter added this to the 3.1.0 milestone Sep 5, 2014
@batter
Copy link
Collaborator

batter commented Sep 23, 2014

I was actually not aware of this, so thanks for bringing this up. I guess all of the functionality we have built around serialized attributes will need to be conditionally removed for ActiveRecord 5 users. Do you have a link to an article or discussion as to reasoning behind this removal? Just curious.

@thetron
Copy link
Author

thetron commented Sep 24, 2014

I only noticed it because I've been building an app on the 4.2 beta. I did have a little bit of a dig around to see if I could find out why it's being removed, but couldn't find much.

There's rails/rails#15467 and rails/rails#15704 which seem to touch on the reasoning behind the removal.

@gshaw
Copy link

gshaw commented Dec 2, 2014

I think this needs to be removed for 4.2 as well because the test suite is very noisy with the deprecation messages. I'm testing my applications with 4.2.0.rc1 and this warning is very noisy.

If somebody can add some pointers on how this should be fixed I might be able to take a look at this.

@theunraveler
Copy link

To silence this warning In the meantime, you can add the following to an initializer file:

current_behavior = ActiveSupport::Deprecation.behavior
ActiveSupport::Deprecation.behavior = lambda do |message, callstack|
  return if message =~ /`serialized_attributes` is deprecated without replacement/ && callstack.any? { |m| m =~ /paper_trail/ }
  Array.wrap(current_behavior).each { |behavior| behavior.call(message, callstack) }
end

That will just silence this one paper_trail message, not all deprecation warnings.

@batter
Copy link
Collaborator

batter commented Dec 2, 2014

What is the long term solution expectation? We just need to RM all of the behavior surrounding serialized attribute handling?

@mecampbellsoup
Copy link

@batter 👍

@senny
Copy link
Contributor

senny commented Dec 8, 2014

I just noticed this while upgrading my apps to 4.2.0.rc2. With Rails being close to a final release it would be nice to see where this is going.

I haven't checked the implementation yet but as mentioned in the PR, there should be a way to get the information from serialized_attributes through other means. I don't think you'll have to drop complete support but probably collect the data differently.

@batter let me know when you need something to move this forward.

@batter
Copy link
Collaborator

batter commented Dec 9, 2014

@senny - serialized_attributes won't be removed until Rails 5, it just spits a bunch of deprecation warnings out with rails 4.2 (as is standard practice when deprecating a feature).

So I've been thinking about how to handle this. I'll have to try to get a sense of the timeline for when they expect Rails 5 to be delivered. Basically, if we want to maintain backwards compatibility, I think we need to check which version of Rails is being used and/or whether serialized_attributes exists on the model instance and react accordingly.

@senny
Copy link
Contributor

senny commented Dec 9, 2014

@batter the feature is deprecated as of 4.2 and will be gone in 5.0. Meaning, it still works under 4.2 but you are encouraged to move to a different solution. Hence the warning messages.

It would be nice to assess what special functionality was built on top of serialized_attributes. With the deprecation there were also refactorings of the internal handling of serialized attributes in general. With the main goal to make serialized attributes behave similar to normal attributes. It's quite possible that this gem will require less specific code to handle serialized attributes.

Imho it would be good to address this for 4.2 without waiting for 5.0.

@batter
Copy link
Collaborator

batter commented Dec 9, 2014

@senny - Right, sorry about throwing around inaccurate terminology. Didn't have coffee or re-read the issue in its entirety when responding earlier today. At any rate, I will work on this next after I get #439 merged. I think it's pretty clear cut what needs to be altered for this as we have specific methods for handling serialized attributes.

Do we know if there is a suggested alternative? @sgrif mentions polymorphism in his PR rails/rails#15704, I wasn't totally clear on whether he was referring to utilizing model STI's or something to that effect.

@sgrif
Copy link
Contributor

sgrif commented Dec 9, 2014

Glancing through the code, it looks like there's not really any reason that you should need special handling for serialized attributes, they should behave consistently enough to avoid issues (serializes on the way to the database, deserializes on the way out). If you're needing to hook into them outside of the context of an AR instance, maybe look at using the type objects we introduced in 4.2 to handle the casting to/from the database for you? That's what I meant by polymorphism.

@batter
Copy link
Collaborator

batter commented Dec 9, 2014

Thanks for the response @sgrif.

This dates back to #180, and the reasons are twofold: 1st was attempting to mimic what ActiveRecord was doing with data, second is due to the fact that we serialize the attributes hash prior to inserting it into the DB, we wanted to guarantee that objects that needed serialization prior to being cast to write-able data were able to be unserialized / read back out in the same format they were at when a version is generated.

Will take a look at those type objects but hopefully we won't even need them.

@sgrif
Copy link
Contributor

sgrif commented Dec 9, 2014

The individual states are much better defined now, as are their transitions. You should be able to get that guarantee from AR automatically now, unless I'm misunderstanding you.

@jesperronn
Copy link

Hi, if I understand the issue correctly, it does not prevent me from upgrading my project to Rails 4.2, its only a warning, (annoying warning), but it still works correctly and does not invalidate my model.

Is this correctly understood?

@senny
Copy link
Contributor

senny commented Dec 22, 2014

@jesperronn yes, that's correct.

@jaredbeck
Copy link
Member

batter closed this in 3a17367 a day ago

Thanks @batter! Should rails 4.2 projects use master for now? Is there a paper_trail 3.0.7 or 3.1 release in the works? Thanks!

@batter
Copy link
Collaborator

batter commented Dec 30, 2014

@jaredbeck - I just released v4.0.0.beta1 to Rubygems, so you can give that a whirl and it should get rid of all the deprecation warnings surrounding serialized_attributes. Also it would be helpful for me to get some feedback on how all the recent changes that have gone into this pending version work for others 😄

For the record: You can also continue using 3.0.x, since those are just warnings, and the gem should still continue to function as normal, you will just see those deprecation warnings that have been discussed in this thread.

@cnk
Copy link

cnk commented Jan 4, 2015

Thanks. The beta appears to work fine on my rails 4.2 project. I needed run rails generate paper_trail:install --with-associations to get the new table and column and then add require 'paper_trail/frameworks/rspec' to spec/rails_helper.rb

@yule
Copy link

yule commented Jan 14, 2015

Beta works fine for me on 4.2. as same as @cnk : had to run generate and require to spec_helper

@klacointe
Copy link

Rails 4.2 and paper_trail 4.0.0.rc2 : 🆗
Thanks 😘

@jasdeepsingh
Copy link

Rails 4.2 with paper_trail 4.0.0.beta2 👍

@tim3z
Copy link

tim3z commented May 8, 2015

after silencing the serialized attributes warning I just noticed, that there was also another similar warning:

DEPRECATION WARNING: `#reset_updated_at!` is deprecated and will be removed on Rails 5. Please use `#restore_updated_at!` instead.

@batter
Copy link
Collaborator

batter commented May 8, 2015

@SDEaGLe - what version of PaperTrail are you using? That message should not appear in version 4.0.0.beta2 and higher.

@tim3z
Copy link

tim3z commented May 8, 2015

latest stable

@stephenbaidu
Copy link

@sgrif, I can't seem to find the "type objects" you mentioned. Some links would be helpful. Thanks.

@sgrif
Copy link
Contributor

sgrif commented May 9, 2015

@stephenbaidu
Copy link

Thanks

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

No branches or pull requests