From ab20f7b1894b121523ee7364c01f502b16f638f6 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Wed, 8 Jul 2009 19:51:45 +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 | 23 +++- .../test/cases/autosave_association_test.rb | 16 ++-- activerecord/test/cases/validations_i18n_test.rb | 106 ++++++++++++++++++++ activerecord/test/schema/schema.rb | 2 + 5 files changed, 135 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..0d65031 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -196,20 +196,31 @@ 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 + begin + I18n.translate "activerecord.attributes.#{arr[0]}.#{arr[1]}", :raise => true + rescue I18n::MissingTranslationData + @base.class.reflections[arr[0].to_sym].class_name.constantize.human_attribute_name arr[1] + end 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