mobility icon indicating copy to clipboard operation
mobility copied to clipboard

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

Open ingemar opened this issue 7 years ago • 9 comments

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

ingemar avatar Dec 18 '17 14:12 ingemar

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 avatar Dec 18 '17 15:12 shioyama

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

shioyama avatar Dec 20 '17 12:12 shioyama

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.

ingemar avatar Dec 21 '17 09:12 ingemar

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

shioyama avatar Dec 21 '17 09:12 shioyama

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.

shioyama avatar Dec 21 '17 09:12 shioyama

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.

ingemar avatar Dec 21 '17 14:12 ingemar

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?

ruby-fu-ninja avatar Jul 03 '18 07:07 ruby-fu-ninja

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.

shioyama avatar Jul 03 '18 08:07 shioyama

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

ruby-fu-ninja avatar Jul 03 '18 22:07 ruby-fu-ninja

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

shioyama avatar May 27 '23 09:05 shioyama