Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• 

  José Valim September 7th, 2010 @ 02:54 PM

  • Milestone cleared.
  • Importance changed from “” to “Low”

  It looks good, although I would prefer to add a new value under model_name (see ActiveModel::Naming) called @i18n_key instead of doing it manually inside human. We probably need to change human_attribute_name as well.

• You flagged this item as spam.
  

  Rodrigo Rosenfeld Rosas September 7th, 2010 @ 04:17 PM

  Take a look if this is what you are talking about. And please also comment on #3768...

  Thanks

  • fix-class-with-module-i18n-scope-v2.patch 5.3 KB
• 

  José Valim September 7th, 2010 @ 04:36 PM

  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:

• You flagged this item as spam.
  

  Rodrigo Rosenfeld Rosas September 7th, 2010 @ 04:42 PM

  Have fun! :)

• 

  José Valim September 18th, 2010 @ 08:21 PM

  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!

• You flagged this item as spam.
  

  Rodrigo Rosenfeld Rosas September 18th, 2010 @ 10:18 PM

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

  • fix-class-with-module-i18n-scope-v3.patch 5.2 KB
• 

  José Valim September 18th, 2010 @ 11:13 PM

  Yup, it looks great! :D

• You flagged this item as spam.
  

  StarPeak September 21st, 2010 @ 07:32 PM

  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 ;)

  • fix-class-with-module-i18n-scope-v3-with-fallback.patch 4.8 KB
• 

  José Valim September 24th, 2010 @ 12:07 PM

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

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

  Thanks.

• 

  José Valim September 24th, 2010 @ 12:07 PM

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

• You flagged this item as spam.
  

  Rodrigo Rosenfeld Rosas September 24th, 2010 @ 01:14 PM

  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 :)

• 

  José Valim September 24th, 2010 @ 01:19 PM

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

• You flagged this item as spam.
  

  Rodrigo Rosenfeld Rosas September 24th, 2010 @ 05:02 PM

  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... :(

• 

  José Valim September 24th, 2010 @ 07:42 PM

  • State changed from “new” to “resolved”

  Rodrigo, just applied your patch. But please use git format-patch in the next ones! :D Thanks!

• 

  Repository September 24th, 2010 @ 07:46 PM

  (from [8d30193b08bd2321a7a78a1f481bd5e4d4d45557]) Properly interpolate i18n keys in modules [#5572 state:resolved] http://github.com/rails/rails/commit/8d30193b08bd2321a7a78a1f481bd5...

• 

  Repository September 24th, 2010 @ 07:50 PM

  (from [8d14fa89599a30a6ab1d14faedd2de8e46651272]) Properly interpolate i18n keys in modules [#5572 state:resolved]

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

• You flagged this item as spam.
  

  Rodrigo Rosenfeld Rosas September 24th, 2010 @ 10:54 PM

  Ok, by the way, what is the status of #3768

• You flagged this item as spam.
  

  StarPeak September 25th, 2010 @ 12:40 PM

  Now in format following the conventions in the contributor guide (

  • fix-class-with-module-i18n-scope-with-fallback.patch 5.9 KB

• You flagged this item as spam.
  

  Rodrigo Rosenfeld Rosas September 25th, 2010 @ 02:07 PM

  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

• You flagged this item as spam.
  

  Rodrigo Rosenfeld Rosas September 25th, 2010 @ 02:12 PM

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

• You flagged this item as spam.
  

  StarPeak September 25th, 2010 @ 06:02 PM

  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'

  • fix-class-with-module-i18n-scope-v4.patch 9.5 KB

• You flagged this item as spam.
  

  Rodrigo Rosenfeld Rosas September 25th, 2010 @ 08:46 PM

  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

• 

  Jeremy Kemper October 15th, 2010 @ 11:02 PM

  • Milestone set to “3.0.2”

  [bulk edit]

• 

  Repository February 27th, 2011 @ 11:44 PM

  (from [a00bed0c48f469d1cd2de364bfaddbc724046195]) Revert "Properly interpolate i18n keys in modules [#5572 state:resolved]"

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

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

• 

  Repository February 27th, 2011 @ 11:49 PM

  (from [f80eea3bf30dcceaca3c4a9cd9238bdc44a9e165]) Revert "Properly interpolate i18n keys in modules [#5572 state:resolved]"

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

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

• 

  Santiago Pastorino February 28th, 2011 @ 02:11 AM

  • State changed from “resolved” to “invalid”
• 

  Repository March 31st, 2011 @ 06:28 AM

  (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, #5572] https://github.com/rails/rails/commit/0307c538eca5a3b41d326d1357f09...

• 

  Repository March 31st, 2011 @ 06:28 AM

  (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, #5572] https://github.com/rails/rails/commit/5b8dbb0eee0a3277ef51e4bf31555...