diff options
-rw-r--r-- | activerecord/CHANGELOG | 3 | ||||
-rwxr-xr-x | activerecord/lib/active_record/validations.rb | 16 | ||||
-rw-r--r-- | activerecord/test/fixtures/db_definitions/schema.rb | 6 | ||||
-rwxr-xr-x | activerecord/test/validations_test.rb | 45 |
4 files changed, 68 insertions, 2 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 796c7274a7..26e0a7f9c2 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -29,7 +29,8 @@ * MySQL: fix change_column on not-null columns that don't accept dfeault values of ''. #6663 [Jonathan Viney, Tarmo Tänav] -* validates_uniqueness_of behaves well with single-table inheritance. #3833 [Gabriel Gironda, rramdas, François Beausoleil, Josh Peek, Tarmo Tänav] +* validates_uniqueness_of behaves well with abstract superclasses and +single-table inheritance. #3833, #9886 [Gabriel Gironda, rramdas, François Beausoleil, Josh Peek, Tarmo Tänav, pat] * Raise ProtectedAttributeAssignmentError in development and test environments when mass-assigning to an attr_protected attribute. #9802 [Henrik N] diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 6514c4077a..35a1f8d0d4 100755 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -633,7 +633,21 @@ module ActiveRecord condition_params << record.send(:id) end - if find(:first, :conditions => [condition_sql, *condition_params]) + # The check for an existing value should be run from a class that + # isn't abstract. This means working down from the current class + # (self), to the first non-abstract class. Since classes don't know + # their subclasses, we have to build the hierarchy between self and + # the record's class. + class_hierarchy = [record.class] + while class_hierarchy.first != self + class_hierarchy.insert(0, class_hierarchy.first.superclass) + end + + # Now we can work our way down the tree to the first non-abstract + # class (which has a database table to query from). + finder_class = class_hierarchy.detect { |klass| !klass.abstract_class? } + + if finder_class.find(:first, :conditions => [condition_sql, *condition_params]) record.errors.add(attr_name, configuration[:message]) end end diff --git a/activerecord/test/fixtures/db_definitions/schema.rb b/activerecord/test/fixtures/db_definitions/schema.rb index 88f99f51ac..dcb6222cd3 100644 --- a/activerecord/test/fixtures/db_definitions/schema.rb +++ b/activerecord/test/fixtures/db_definitions/schema.rb @@ -289,4 +289,10 @@ ActiveRecord::Schema.define do t.column :message, :string, :null=>false t.column :developer_id, :integer, :null=>false end + + create_table :inept_wizards, :force => true do |t| + t.column :name, :string, :null => false + t.column :city, :string, :null => false + t.column :type, :string + end end diff --git a/activerecord/test/validations_test.rb b/activerecord/test/validations_test.rb index 49fc30eff7..2f67e9a36d 100755 --- a/activerecord/test/validations_test.rb +++ b/activerecord/test/validations_test.rb @@ -33,6 +33,22 @@ class Topic < ActiveRecord::Base has_many :silly_unique_replies, :dependent => :destroy, :foreign_key => "parent_id" end +class Wizard < ActiveRecord::Base + self.abstract_class = true + + validates_uniqueness_of :name +end + +class IneptWizard < Wizard + validates_uniqueness_of :city +end + +class Conjurer < IneptWizard +end + +class Thaumaturgist < IneptWizard +end + class ValidationsTest < Test::Unit::TestCase fixtures :topics, :developers @@ -402,6 +418,35 @@ class ValidationsTest < Test::Unit::TestCase assert t2.save, "should save with nil" end + def test_validate_straight_inheritance_uniqueness + w1 = IneptWizard.create(:name => "Rincewind", :city => "Ankh-Morpork") + assert w1.valid?, "Saving w1" + + # Should use validation from base class (which is abstract) + w2 = IneptWizard.new(:name => "Rincewind", :city => "Quirm") + assert !w2.valid?, "w2 shouldn't be valid" + assert w2.errors.on(:name), "Should have errors for name" + assert_equal "has already been taken", w2.errors.on(:name), "Should have uniqueness message for name" + + w3 = Conjurer.new(:name => "Rincewind", :city => "Quirm") + assert !w3.valid?, "w3 shouldn't be valid" + assert w3.errors.on(:name), "Should have errors for name" + assert_equal "has already been taken", w3.errors.on(:name), "Should have uniqueness message for name" + + w4 = Conjurer.create(:name => "The Amazing Bonko", :city => "Quirm") + assert w4.valid?, "Saving w4" + + w5 = Thaumaturgist.new(:name => "The Amazing Bonko", :city => "Lancre") + assert !w5.valid?, "w5 shouldn't be valid" + assert w5.errors.on(:name), "Should have errors for name" + assert_equal "has already been taken", w5.errors.on(:name), "Should have uniqueness message for name" + + w6 = Thaumaturgist.new(:name => "Mustrum Ridcully", :city => "Quirm") + assert !w6.valid?, "w6 shouldn't be valid" + assert w6.errors.on(:city), "Should have errors for city" + assert_equal "has already been taken", w6.errors.on(:city), "Should have uniqueness message for city" + end + def test_validate_format Topic.validates_format_of(:title, :content, :with => /^Validation\smacros \w+!$/, :message => "is bad data") |