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

When nested object is namespaced, i18n key is invalid ("namespaced" too) #900

Closed
stevo opened this issue Nov 13, 2012 · 10 comments
Closed
Labels
Milestone

Comments

@stevo
Copy link

stevo commented Nov 13, 2012

i.e.

Shop::Project has_many Shop::Products

instead of looking for

en: 
 formtastic:
  labels:
   project:
    product:

it will look for

en: 
 formtastic:
  labels:
   project:
    shop/product: 

Probably changing

model_name, nested_model_name = normalize_model_name(self.model_name.underscore)

to

model_name, nested_model_name = normalize_model_name(self.model_name.demodulize.underscore)

should solve the problem

@justinfrench
Copy link
Member

Would love to see a pull request for this!

@justinfrench
Copy link
Member

This one is going to be tricky! I've had a play around in the code, but I think we first need a decent plan of attack.

We'll need to decide if it's best to remove the namespace as suggested above, or include it in a better way. Using the example above, we currently look for labels.project.shop/product. Alternatives might include ignoring the namespace (labels.project.product) as @stevo suggested, or using it as part of the key (e.g. labels.project.shop.product).

Assuming we have a solid view on what the right key is, we might need to support the existing behaviour (labels.project.shop/product) as well (am I right that this currently "works", but isn't ideal?) and deprecate it over time. If the current way simply doesn't work, that'd be nice :)

I've bumped this out of 2.3.0 as a work-around exists and was documented in #964, and because I don't think there's a solid plan here yet.

@sobrinho?

@justinfrench
Copy link
Member

Proposed plan forward for the above labels.project.shop/product example:

  1. search labels.project.shop.product first
  2. fallback to labels.project.product
  3. warn or deprecate labels.project.shop/product

Given @stevo reported that labels.project.shop/product reported that this is an invalid key, it doesn't feel like there's any deprecation to be done. It's just a fix which could be applied to 2.3-stable, 3.0-stable and master.

Any thoughts before I try to implement?

@justinfrench
Copy link
Member

SimpleForm seems to have landed on labels.project.shop_product to avoid multiple lookups, so I see no reason not to take the same approach at this point.

@justinfrench justinfrench modified the milestones: 3.1, 3.0 final Sep 6, 2014
@mikz
Copy link
Contributor

mikz commented Nov 1, 2014

Rails is using slashes in i18 keys. https://github.com/rails/rails/blob/master/activemodel/lib/active_model/naming.rb#L158
Nesting them by dots proved difficult and had to be changed in 3.2. (like https://github.com/rails/rails/pull/3859/files).

I would be for using the absolute names everywhere. It is obvious and explicit.

@justinfrench justinfrench modified the milestones: 3.2, 3.1 Nov 17, 2014
@justinfrench
Copy link
Member

@mikz it isn't clear to me how to get this resolved. If Rails supports slashes, feels like we should do the same. If we choose to do the same, do we need to do any work?

@mikz
Copy link
Contributor

mikz commented Nov 22, 2014

Considering the original issue, I think Rails would try: en.formtastic.labels.shop/project.shop/product.
We would review if Formtastic is using ActiveModel::Naming#i18n_key everywhere and if not, use it.
That should keep it consistent with upcoming Rails versions.

@justinfrench
Copy link
Member

@mikz would there be any change in Formtastic behaviour that you're aware of? If there's likely to be any subtle changes, I'd prefer to bump this out to 4.0. If it's okay for 3.2, maybe I'll start by adding tests to check existing behaviour?

@mikz
Copy link
Contributor

mikz commented Nov 22, 2014

I18n allows fallbacks which could be used, so for sake of gradual change it should be done in phases. It would be hard to how a deprecation message though. But the final change should be definitely for 4.0. The 3.2 would be just adding support for the 'rails consistent' mode. (unless it is already consistent, dunno).

@justinfrench
Copy link
Member

I just tried this with Formtastic master (~3.1.1) and Rails 4.1.6:

#en.yml
en:
  formtastic:
    labels:
      nested/post:
        title: Foo

# app/views/nested/posts/new.html.erb
<%= semantic_form_for @nested_post do |f| %>
  <%= f.inputs :title %>
<% end %>

The form correctly rendered the label for the :title as "Foo", there's no deprecation or i18n key errors, and this behaviour is consistent with how Rails translates nested models, so I'm closing this issue for now.

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

No branches or pull requests

3 participants