rails_lh2210_attachment.diff

From dea2c88948364c5f6e6f853ff8cdd0fcd68158d0 Mon Sep 17 00:00:00 2001
From: Akira Matsuda <ronnie@dio.jp>
Date: Thu, 9 Jul 2009 16:08:59 +0900
Subject: [PATCH] Fix validation on associations to process I18n aware error messages

---
 .../lib/active_record/autosave_association.rb      |    4 +-
 activerecord/lib/active_record/validations.rb      |   24 ++++-
 .../test/cases/autosave_association_test.rb        |   16 ++--
 activerecord/test/cases/validations_i18n_test.rb   |  106 ++++++++++++++++++++
 activerecord/test/schema/schema.rb                 |    2 +
 5 files changed, 136 insertions(+), 16 deletions(-)

diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index 9717ca3..791840b 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -250,7 +250,7 @@ module ActiveRecord
         if reflection.options[:autosave]
           unless association.marked_for_destruction?
             association.errors.each do |attribute, message|
-              attribute = "#{reflection.name}_#{attribute}"
+              attribute = "#{reflection.name}.#{attribute}"
               errors.add(attribute, message) unless errors.on(attribute)
             end
           end
@@ -350,4 +350,4 @@ module ActiveRecord
       end
     end
   end
-end
\ No newline at end of file
+end
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index d2d12b8..a9a5f44 100644
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -196,20 +196,32 @@ module ActiveRecord
     def full_messages(options = {})
       full_messages = []
 
-      @errors.each_key do |attr|
-        @errors[attr].each do |message|
+      @errors.each_key do |key|
+        @errors[key].each do |message|
           next unless message
 
-          if attr == "base"
+          arr = key.split('.')
+          if (key == 'base') || (arr[1] == 'base')
             full_messages << message
+            next
+          end
+
+          translated_attribute_name = if arr[1].nil?
+            @base.class.human_attribute_name key
           else
-            attr_name = @base.class.human_attribute_name(attr)
-            full_messages << attr_name + I18n.t('activerecord.errors.format.separator', :default => ' ') + message
+            defaults = []
+            reflection = @base.class.reflections[arr[0].to_sym]
+            defaults << :"#{reflection.class_name.underscore}.#{arr[1]}" if reflection
+            defaults << arr[1].humanize
+
+            I18n.translate :"#{arr[0]}.#{arr[1]}", :scope => 'activerecord.attributes', :default => defaults
           end
+
+          full_messages << translated_attribute_name + I18n.t('activerecord.errors.format.separator', :default => ' ') + message
         end
       end
       full_messages
-    end 
+    end
 
     # Returns true if no errors have been added.
     def empty?
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index 919b6f8..d82e118 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -646,14 +646,14 @@ class TestAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase
   def test_should_automatically_validate_the_associated_model
     @pirate.ship.name = ''
     assert !@pirate.valid?
-    assert !@pirate.errors.on(:ship_name).blank?
+    assert !@pirate.errors.on('ship.name').blank?
   end
 
   def test_should_merge_errors_on_the_associated_models_onto_the_parent_even_if_it_is_not_valid
     @pirate.ship.name   = nil
     @pirate.catchphrase = nil
     assert !@pirate.valid?
-    assert !@pirate.errors.on(:ship_name).blank?
+    assert !@pirate.errors.on('ship.name').blank?
     assert !@pirate.errors.on(:catchphrase).blank?
   end
 
@@ -736,7 +736,7 @@ class TestAutosaveAssociationOnABelongsToAssociation < ActiveRecord::TestCase
   def test_should_automatically_validate_the_associated_model
     @ship.pirate.catchphrase = ''
     assert !@ship.valid?
-    assert !@ship.errors.on(:pirate_catchphrase).blank?
+    assert !@ship.errors.on('pirate.catchphrase').blank?
   end
 
   def test_should_merge_errors_on_the_associated_model_onto_the_parent_even_if_it_is_not_valid
@@ -744,7 +744,7 @@ class TestAutosaveAssociationOnABelongsToAssociation < ActiveRecord::TestCase
     @ship.pirate.catchphrase = nil
     assert !@ship.valid?
     assert !@ship.errors.on(:name).blank?
-    assert !@ship.errors.on(:pirate_catchphrase).blank?
+    assert !@ship.errors.on('pirate.catchphrase').blank?
   end
 
   def test_should_still_allow_to_bypass_validations_on_the_associated_model
@@ -806,7 +806,7 @@ module AutosaveAssociationOnACollectionAssociationTests
     @pirate.send(@association_name).each { |child| child.name = '' }
 
     assert !@pirate.valid?
-    assert_equal "can't be blank", @pirate.errors.on("#{@association_name}_name")
+    assert_equal "can't be blank", @pirate.errors.on("#{@association_name}.name")
     assert @pirate.errors.on(@association_name).blank?
   end
 
@@ -814,7 +814,7 @@ module AutosaveAssociationOnACollectionAssociationTests
     @pirate.send(@association_name).build(:name => '')
 
     assert !@pirate.valid?
-    assert_equal "can't be blank", @pirate.errors.on("#{@association_name}_name")
+    assert_equal "can't be blank", @pirate.errors.on("#{@association_name}.name")
     assert @pirate.errors.on(@association_name).blank?
   end
 
@@ -823,7 +823,7 @@ module AutosaveAssociationOnACollectionAssociationTests
     @pirate.catchphrase = nil
 
     assert !@pirate.valid?
-    assert_equal "can't be blank", @pirate.errors.on("#{@association_name}_name")
+    assert_equal "can't be blank", @pirate.errors.on("#{@association_name}.name")
     assert !@pirate.errors.on(:catchphrase).blank?
   end
 
@@ -920,4 +920,4 @@ class TestAutosaveAssociationOnAHasAndBelongsToManyAssociation < ActiveRecord::T
   end
 
   include AutosaveAssociationOnACollectionAssociationTests
-end
\ No newline at end of file
+end
diff --git a/activerecord/test/cases/validations_i18n_test.rb b/activerecord/test/cases/validations_i18n_test.rb
index 6698234..7e90d63 100644
--- a/activerecord/test/cases/validations_i18n_test.rb
+++ b/activerecord/test/cases/validations_i18n_test.rb
@@ -1,6 +1,9 @@
 require "cases/helper"
 require 'models/topic'
 require 'models/reply'
+require 'models/pirate'
+require 'models/ship'
+require 'models/ship_part'
 
 class ActiveRecordValidationsI18nTests < ActiveSupport::TestCase
   def setup
@@ -911,5 +914,108 @@ class ActiveRecordValidationsGenerateMessageI18nTests < Test::Unit::TestCase
   def test_generate_message_even_with_default_message
     assert_equal "must be even", @topic.errors.generate_message(:title, :even, :default => nil, :value => 'title', :count => 10)
   end
+end
+
+class ActiveRecordValidationsOnAutosaveAssociationOnI18nTests < ActiveRecord::TestCase
+  def setup
+    @old_load_path, @old_backend = I18n.load_path, I18n.backend
+    I18n.load_path.clear
+    I18n.backend = I18n::Backend::Simple.new
+    I18n.backend.store_translations('en', :activerecord => {
+      :errors => {:messages => {:blank => "can't be blank", :custom => nil}},
+      :attributes => {
+        :ship => {:weight => 'Weight'}}})
+  end
+
+  def teardown
+    I18n.load_path.replace @old_load_path
+    I18n.backend = @old_backend
+  end
+
+  test 'validation error on autosave association on a has_one association translates attribute name' do
+    repair_validations Ship do
+      Ship.validates_presence_of :weight
+
+      pirate = Pirate.new :catchphrase => "I'm Gonna Be King of the Pirates!"
+      ship = pirate.build_ship :name => 'Going Merry'
+      pirate.valid?
+      assert_equal ["Weight can't be blank"], pirate.errors.full_messages
+    end
+  end
+
+  test 'validation error on autosave association on a has_many association translates attribute name' do
+    repair_validations ShipPart do
+      ShipPart.validates_presence_of :material
+
+      ship = Ship.new :name => 'Going Merry'
+      part = ship.parts.build :name => 'mast'
+      ship.valid?
+      assert_equal ["Material can't be blank"], ship.errors.full_messages
+    end
+  end
+
+  test 'validation error on autosave association on a belongs_to association translates attribute name' do
+    ship = Ship.new :name => 'Thousand Sunny'
+    ship.build_pirate
+    ship.valid?
+    assert_equal ["Catchphrase can't be blank"], ship.errors.full_messages
+  end
+end
+
+class ActiveRecordValidationsOnAutosaveAssociationOnI18nFallbackTests < ActiveRecord::TestCase
+  def setup
+    @old_load_path, @old_backend = I18n.load_path, I18n.backend
+    I18n.load_path.clear
+    I18n.backend = I18n::Backend::Simple.new
+    I18n.backend.store_translations('en', :activerecord => {
+      :errors => {:messages => {:blank => "can't be blank", :custom => nil}},
+      :attributes => {
+        :ship_part => {:material => 'Material'},
+        :parts => {:material => 'Part material'}}})
+  end
 
+  def teardown
+    I18n.load_path.replace @old_load_path
+    I18n.backend = @old_backend
+  end
+
+  test 'validation error on autosave association translates attribute name by association name first, and then by model name' do
+    repair_validations ShipPart do
+      ShipPart.validates_presence_of :material
+
+      ship = Ship.new :name => 'Going Merry'
+      part = ship.parts.build :name => 'mast'
+      ship.valid?
+      assert_equal ["Part material can't be blank"], ship.errors.full_messages
+
+      I18n.backend.send(:translations)[:en][:activerecord][:attributes].delete :parts
+      ship.valid?
+      assert_equal ["Material can't be blank"], ship.errors.full_messages
+    end
+  end
+end
+
+class ActiveRecordValidationsOnAutosaveAssociationOnI18nBaseErrorsTests < ActiveRecord::TestCase
+  def setup
+    Pirate.class_eval do
+      def validate
+        errors.add :base, "Hey, there's something wrong with this pirate!"
+      end
+    end
+  end
+
+  def teardown
+    Pirate.class_eval do
+      def validate; end
+    end
+  end
+
+  test 'validation error on autosave association does not translate attribute name for base attribute' do
+    repair_validations Pirate do
+      ship = Ship.new :name => 'Thousand Sunny'
+      pirate = ship.build_pirate :catchphrase => "I'm Gonna Be King of the Pirates!"
+      ship.valid?
+      assert_equal ["Hey, there's something wrong with this pirate!"], ship.errors.full_messages
+    end
+  end
 end
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 98e6d19..6e58498 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -373,6 +373,7 @@ ActiveRecord::Schema.define do
 
   create_table :ships, :force => true do |t|
     t.string :name
+    t.integer :weight
     t.integer :pirate_id
     t.datetime :created_at
     t.datetime :created_on
@@ -382,6 +383,7 @@ ActiveRecord::Schema.define do
 
   create_table :ship_parts, :force => true do |t|
     t.string :name
+    t.string :material
     t.integer :ship_id
   end
 
-- 
1.6.3.3