[PATCH] Fix i18n scope bug when class belongs to a module

Take a look if this is what you are talking about. And please also comment on #3768 Add :full_message option to validations...

Thanks

It looks great! But I'm going on vacation now, I will apply it in 2
weeks. :)

On Sep 7, 2010, at 17:17, "Lighthouse" <no-reply@lighthouseapp.com>
wrote:

Have fun! :)

Sorry Rodrigo, looking better at your patch I saw you added the i18n_key method in the wrong place.

It is supposed to be invoked as klass.model_name.i18n_key and not to be a method added straight to the class. Do you think you can provide a new patch?

Thanks!

Please, take a look if this is what you were talking about.

Yup, it looks great! :D

I just added fallback to modules translations for errors and attributes if none were given for the namespaced model itself.

So e.g. PersonModule::Person.human_attribute_name('title') will lookup en.activemodel.attributes.person_module.title if en.activemodel.attributes.person_module.person.title was not found.

Tests are running fine ;)

StarPeak, could you please provide a patch following the conventions in the contributor guide?

http://rails.lighthouseapp.com/projects/8994/sending-patches

Thanks.

Rodrigo, you also need to use the conventions in the contributor guide in order to keep you as the commit author.

I could do that when I get home but it won't make much sense if the patch with fallback will be applied instead... Anyway, for simple patches like this, I wouldn't mind to "git commit --author='Rodrigo Rosenfeld Rosas rr_rosas@yahoo.com.br'". Tell me if you would like me to send you the mail formatted patch when I get home. I didn't even commit the changes (just git diff > file.patch), so the commit date will be tonight :)

The fallback patch will be applied after yours. Yes, I can apply the author as you proposed! :)

I've just noted LH messed with the git commit command, but I guess you got the idea. Just in case, let-me repeat the command:

git commit --author="Rodrigo Rosenfeld Rosas <rr_rosas@yahoo.com.br>"

I need to get used to use the preview feature of LH since I often have problems like that... :(

(from [8d30193b08bd2321a7a78a1f481bd5e4d4d45557]) Properly interpolate i18n keys in modules [#5572 [PATCH] Fix i18n scope bug when class belongs to a module state:resolved] http://github.com/rails/rails/commit/8d30193b08bd2321a7a78a1f481bd5...

(from [8d14fa89599a30a6ab1d14faedd2de8e46651272]) Properly interpolate i18n keys in modules [#5572 [PATCH] Fix i18n scope bug when class belongs to a module state:resolved]

Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/8d14fa89599a30a6ab1d14faedd2de...

Ok, by the way, what is the status of #3768 Add :full_message option to validations

Now in format following the conventions in the contributor guide (

Hi StarPeak, I would like to state some comments:

First, please correct the typo in your patch (test_transalted...).

I don't think there is a need for separating default attribute types from default types. I would do something like:

defaults = @base.class.lookup_ancestors.inject([]) do |defaults, klass|
  key_parts = klass.model_name.i18n_key.to_s.split('.')
  while ! key_parts.empty?
    defaults << :"#{@base.class.i18n_scope}.errors.models.#{key_parts.join '.'}.attributes.#{attribute}.#{type}"
    defaults << :"#{@base.class.i18n_scope}.errors.models.#{key_parts.join '.'}.#{type}"
    key_parts.pop
  end
end

But please don't change anything (except the typo) before listening to Valim's opinions

Actually, I've missed a "defaults" at the end of the block :)

I fixed the typo and took your .empty? idea, as it is better readable.

I think the separation of the two types of errors makes sense as the lookup order should be:

.person_module.person.attributes.name. .person_module.attributes.name. .person_module.person. .person_module.

Instead of:

.person_module.person.attributes.name. .person_module.person. .person_module.attributes.name. .person_module.

E.g.:

activemodel.errors.models.person_module.attributes.name.too_fancy: 'is a too fancy name.'

should be tried before

activemodel.errors.models.person_module.person.too_fancy: 'is too fancy for a person'

This remembers me that your patch is lacking a documentation update as well as more tests for making sure the lookup order is happening as expected... Otherwise, a later refactor of this code block could change the intended order and no test would fail, alerting about this change...

But, anyway, if you think the proposed order makes more sense, that's ok since I don't have any opinion on how the order should be... Maybe Valim has some other considerations...

Thank you

(from [a00bed0c48f469d1cd2de364bfaddbc724046195]) Revert "Properly interpolate i18n keys in modules [#5572 [PATCH] Fix i18n scope bug when class belongs to a module state:resolved]"

This breaks #6448 i18n key collision with namespaced models, you should use :"module/class" as key for namespacing
[#6448 i18n key collision with namespaced models state:committed]

This reverts commit 8d30193b08bd2321a7a78a1f481bd5e4d4d45557.
https://github.com/rails/rails/commit/a00bed0c48f469d1cd2de364bfadd...

(from [f80eea3bf30dcceaca3c4a9cd9238bdc44a9e165]) Revert "Properly interpolate i18n keys in modules [#5572 [PATCH] Fix i18n scope bug when class belongs to a module state:resolved]"

This breaks #6448 i18n key collision with namespaced models, you should use :"module/class" as key for namespacing
[#6448 i18n key collision with namespaced models state:committed]

This reverts commit 8d14fa89599a30a6ab1d14faedd2de8e46651272.
https://github.com/rails/rails/commit/f80eea3bf30dcceaca3c4a9cd9238...

(from [0307c538eca5a3b41d326d1357f09e4e4f5f3e71]) Support both conventions for translations for namespaced models.

3.0.0 - 3.0.1 required 'namespace/model'
3.0.2 - 3.0.5 required 'namespace.model' (nested). It has the advantage of
keeping the i18n file DRY when multiple models are in the same namespace,
but can lead to translation key conflicts if models are nested within
models.

[#6448 i18n key collision with namespaced models, #5572 [PATCH] Fix i18n scope bug when class belongs to a module] https://github.com/rails/rails/commit/0307c538eca5a3b41d326d1357f09...

(from [5b8dbb0eee0a3277ef51e4bf31555ef27410272f]) Support both conventions for translations for namespaced models.

3.0.0 - 3.0.1 required 'namespace/model'
3.0.2 - 3.0.5 required 'namespace.model' (nested). It has the advantage of
keeping the i18n file DRY when multiple models are in the same namespace,
but can lead to translation key conflicts if models are nested within
models.

[#6448 i18n key collision with namespaced models, #5572 [PATCH] Fix i18n scope bug when class belongs to a module] https://github.com/rails/rails/commit/5b8dbb0eee0a3277ef51e4bf31555...