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

Doesn't work with ActiveRecord::AttributeMethods::Serialization #144

Closed
ingemar opened this issue Dec 18, 2017 · 10 comments
Closed

Doesn't work with ActiveRecord::AttributeMethods::Serialization #144

ingemar opened this issue Dec 18, 2017 · 10 comments

Comments

@ingemar
Copy link

ingemar commented Dec 18, 2017

When using ActiveRecord::AttributeMethods::Serialization, columns doesn't get serialized properly.

@shioyama
Copy link
Owner

Yeah, I know Globalize does this, but I seem to remember the implementation is kind of nasty and breaks with AR releases. I'm not really eager to support this unless it's simple and clean.

@shioyama
Copy link
Owner

Can you explain exactly what you're doing with serialization? Do you want to translate a serialized attribute?

@ingemar
Copy link
Author

ingemar commented Dec 21, 2017

Yes, I'm adding i18n to an existing project and stumbled upon this

# Rails 4.2.10
class SomeModel < ActiveRecord::Base
  serialize :aliases
end

So, a text column serialized to Array with ActiveRecord::Coders::YAMLColumn.

I'm really time strapped here so right now I'm running with this ugly hack, replacing .serialize:

class SomeModel < ActiveRecord::Base
  def self.aliases_coder
    @coder ||= ActiveRecord::Coders::YAMLColumn.new Array
  end

  def aliases=(array)
    aliases_backend.write(Mobility.locale, self.class.aliases_coder.dump(array))
    array
  end

  def aliases(options = {})
    options[:locale] ||= Mobility.locale
    self.class.aliases_coder.load(aliases_backend.read(options.delete(:locale), options)) || []
  end
end

Seem to work for now.

@shioyama
Copy link
Owner

Sorry, to clarify: you're not translating aliases? I don't see why Mobility would even be involved here.

@shioyama
Copy link
Owner

Oh I see, you want to translate serialized arrays...

My position is that if the problem is relevant, and it can be solved in a backend-independent way without hacking into AR internals, it might be a good idea. In this case it seems like it might be possible.

@ingemar
Copy link
Author

ingemar commented Dec 21, 2017

Yes, correct. aliases is just the attribute name for the collection I need to translate.

The reason I haven't submitted a pull request is because I'm not sure what Mobility internals to use.

@ruby-fu-ninja
Copy link

I am also stuck with this problem after trying to move away from Globalize, @shioyama do you have any thoughts on how this problem could be solved?

@shioyama
Copy link
Owner

shioyama commented Jul 3, 2018

Sorry I haven't had time to look at this. I will say that I am very wary of adding special code to handle ActiveRecord features, since this is what makes Globalize so fragile and complicated. I'd prefer to keep things simple and focus on core features, which is why I haven't looked at this. If it involves stuff that may break with a Rails upgrade, I'll be pretty wary about introducing it since I don't want to have to deal with upgrading patches. (e.g. the dirty plugin always causes problems when Rails is upgraded. I'm keeping it since it's very popular.)

I can help someone implement this if you want to take a go at it. The basic steps would be:

  • figure out what Globalize is doing and/or how ActiveRecord handles serialization
  • isolate the part that is specific to the translation table approach in Globalize. I don't think this should be specific to how translations are stored.
  • ideally, if it's independent of how translations are stored, then it can be implemented as a module that can be included in all AR models that use Mobility, like the uniqueness validator.

@ruby-fu-ninja
Copy link

no need to apologise @shioyama

It looks like Globalize inspects the serialized attribute and does a type check for ActiveRecord::Coders::YAMLColumn and in the true case calls serialize on the translated table class
https://github.com/globalize/globalize/blob/f173aacbeee6b63837d00659cd9e577370dc3250/lib/globalize/active_record/act_macro.rb#L86

@shioyama
Copy link
Owner

Closing this, not something Mobility will implement. Happy to provide advice to anyone wanting to implement it as a plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants