Comments (7)
We may need to consider changing from tracking dirty changes on attribute locale accessors, to tracking changes on the attributes themselves.
i.e., instead of what we do currently:
post.title_changes
#=> { "title_en" => [nil, "foo"], "content_en" => [nil, "bar"]}
we would have:
post.title_changes
#=> { "title" => [nil, "foo"], "content" => [nil, "bar"]}
This is, incidentally, what Globalize does. I opted to go with localized accessors so that Mobility could track the full set of locale-specific changes on translated attributes, i.e. if you change the title in English and in French, you will see each change separately. I prefer this approach, but we're running up against problems with how AR is increasingly trying to force everything to specify attribute names upfront, whereas localized names are open-ended (in general, we don't want to assume a set of predefined locales).
That said, currently the dirty plugin depends on fallthrough accessors to actually fetch the localized values of attributes, which uses method_missing
and is thus slow. You can get around that by explicitly defining the localized accessors as methods using locale_accessors: [:en, ...]
for the accessors you actually use, but I suppose many people don't do that and may thus be incurring a performance hit when using dirty...
from mobility.
There currently seems to be three ways to specify locale accessors:
- Explicitly
- Implicitly via
I18n.available_locales
- Dynamically via
fallthrough_accessors
Unless I'm missing something, in case 1 and 2, we could explicitly declare the attributes. So the challenging case to handle is 3.
In my case, my app only supports Japanese and English, and am using the locale specific accessors. I'd have no problem with being required to declare them up front.
The case where you don't want to declare them up front is where your app supports arbitrary translation. In this case, you don't know up front which locales your going to need to support, so you can't declare them. Similarly though, if you don't know what locales your app is going to use, you probably aren't going to be using locale specific accessors for the same reason. Instead, you'd be setting the attribute either implicitly via the current locale (post.title = value
) or explicitly (post.send(:title=, value, locale: :en)
).
This leads me to wonder if there really is a use case that would require someone to use fallthrough_accessors
, and there isn't, perhaps we could just deprecate them.
from mobility.
You're right that it's an edge case we might not want to put too much emphasis on. I guess I like the design of having both fallthrough + explicit locales, whereby you can define explicit locales and yet still fallthrough to any locale.
A use case for this is for example, your app supports mainly a small set of locales, which you set explicitly, but you also want to allow support for new locales organically, perhaps with a performance hit at first.
In the case of the Dirty plugin, fallthrough was (I thought) a nice way to allow the plugin to be used without requiring the user to explicitly specify the locales they want to support upfront (it will work with or without them).
If we did not have fallthrough accessors, I suppose what we would be able to do would be in _read_attribute
just to check if the attr
is included in attributes with all available locales, which is a fixed array so should not be too heavy performance-wise. With the current spec, we'd need to do a regex match like this one, or maybe something a bit simpler, but my concern is that whereas the current regex is checked in method_missing
(so already we're in slow-land), if we put that in _read_attribute
then it impacts every attribute fetch.
So I'm not really happy with this. The annoying thing is that everything works fine for ActiveModel, since AM still simply calls __send__
. It's ActiveRecord overriding _read_attribute
that is the problem.
from mobility.
A use case for this is for example, your app supports mainly a small set of locales, which you set explicitly, but you also want to allow support for new locales organically, perhaps with a performance hit at first.
But do we need explicit accessors for these organic locales? If your app isn't doing something explicitly with Portuguese (for instance), why would you have code like post.title_pt
in it? I could see you doing something like iterating through all locales, but then wouldn't your code look like post.title(locale: locale)
?
In the case of the Dirty plugin, fallthrough was (I thought) a nice way to allow the plugin to be used without requiring the user to explicitly specify the locales they want to support upfront (it will work with or without them).
Agreed that we don't want to require users to explicitly specify locales to do dirty tracking.
from mobility.
So here's one solution: 9b55ccf
What I do is to first check if @attributes
has the attribute, which will be the case for any column. This avoids checking the regex if we are simply fetching a "normal" attribute. If the attribute is not a key on @attributes
, then we resort to first check if it is a translated attribute (without a locale), which is just a check for an element in an array. If we don't find it there, we match against the regex as a last resort, and if it's not there we again fallthrough to super
.
This is not a solution yet - we're checking the regex once, then calling the method, which (if no method is defined for that locale accessor) will go to method_missing
and again check the regex. To avoid that you could just do what fallthrough accessors is doing direclty in the _read_attribute
override, but at that point the whole point of splitting them up as modules is pretty much lost.
So... need to think about this more I guess.
from mobility.
But do we need explicit accessors for these organic locales? If your app isn't doing something explicitly with Portuguese (for instance), why would you have code like post.title_pt in it? I could see you doing something like iterating through all locales, but then wouldn't your code look like post.title(locale: locale)?
Ah I see, so your point is that we could support open-ended locales, just without supporting explicit locale accessors. That's a good point, and in that case I think it would be natural that you can use the Dirty plugin for locales you want to track, but not for the implicit ones.
from mobility.
I'm leaning toward leaving fallthrough accessors as a plugin, but making Dirty require a fixed set of locales (from the union of I18n.available_locales
and whatever locales are passed into LocaleAccessors
, if it is applied to the attribute). This will require a bit more work though. I think you're right though that this is preferable to making the code more complex to handle a very rare use case.
from mobility.
Related Issues (20)
- Error when ordering and selecting distinct on translated resource HOT 2
- Marks object as dirty even if nothing was changed in RichTextTranslation fields HOT 5
- Cache issue with JSONb (+locale_accessors) & ActiveRecord HOT 2
- Using a query method block does not automatically scope to i18n HOT 1
- Translated inputs are empty after failed form validation in a specific setup HOT 1
- Automatic ActionText `rich_text` detection failing when operating on locale-dependent mobility field HOT 1
- Any accepted way to store images working with active storage ? HOT 1
- Unique Validation didn't work with locale_accessors on every locales HOT 1
- Not compatible with FastGettext - request for string available locale support HOT 2
- Error when installing on Vitess MySQL db HOT 4
- The presence plugin doesn't work as expected HOT 1
- Destruction of record+translations fails, when model IDs are of the type "varchar(36)" (a UUID string) HOT 3
- accessor super method doesn't work with arguments like class.value(locale: :de) HOT 1
- ordering by specific locale HOT 2
- Add type checking plugin reference HOT 3
- eager_load and pluck builds the wrong query HOT 1
- `i18n` scope incompatible with `exists?` HOT 1
- validates presence: true failing with cache active HOT 3
- Empty content row is created in translation table. HOT 1
- No fixtures are generated / No doc on how to make them ourselves HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from mobility.