update_attribute integration buggy about mobility

[shioyama]

Unfortunately, I think this is Rails/AR making the kind of thing that Mobility does (defining dynamic attributes that map to different persistence locations) more difficult. Rails 5.1 already made simply marking attributes as dirty more difficult for AR models, which Mobility gets around by overriding has_attribute?.

I'm hesitant to reach too far into AR and override methods to make stuff "work", since it has a tendency to come back to haunt you later. I'm just trying to understand how mutation trackers work to see if we can do anything safely that would make this work... the alternative would just be to override update_attribute to do something different if the attribute is translated, but this approach is a bit like playing whack-a-mole, since if the core dirty issue is not resolved the same problem will again come up somewhere else.

[pwim]

I personally wouldn't go the overwriting route, as I agree it will lead to maintenance issues going forward. Taking a look at it in more detail, I think Rails current handling is actually a bug. They added a check that is no longer necessary. rails/rails#27780 is a PR to fix this behaviour in Rails, and if it is merged, this should work in mobility.

It also isn't such a big deal if the update_attribute method doesn't work with mobility. Normally, I don't think you'd use it with the kind of attributes you want to translate it, I just happened to be using it in my specs, in a place that I could have used update_attributes instead.

At the moment, if someone was using update_attribute, they'd have to rewrite it from article.update_attribute :title, "foo" to

article.title = "foo"
article.save(validate: false)

Given this, I'll close this issue.

[shioyama]

Thanks! Agree that it's not worth implementing. Hopefully Rails will merge that PR.