Skip to content

Instantly share code, notes, and snippets.

@amatsuda
Created July 10, 2009 00:10
Show Gist options
  • Save amatsuda/144116 to your computer and use it in GitHub Desktop.
Save amatsuda/144116 to your computer and use it in GitHub Desktop.
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment