Skip to content

Commit f7d01ec

Browse files
committed
Fix validates_uniqueness_off :field, :allow_nil => false
Closes (rails#5853) Uniqueness validator was not properly checking if there are any existing records, when value was `nil` and column was text type. `nil` was converted to string, which resulted in queries looking like: ```sql SELECT 1 FROM "posts" WHERE "posts"."title" = '' LIMIT 1 ``` instead of ```sql SELECT 1 FROM "posts" WHERE "posts"."title" IS NULL LIMIT 1 ```
1 parent c520504 commit f7d01ec

File tree

2 files changed

+13
-2
lines changed

2 files changed

+13
-2
lines changed

activerecord/lib/active_record/validations/uniqueness.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ def find_finder_class_for(record) #:nodoc:
5454

5555
def build_relation(klass, table, attribute, value) #:nodoc:
5656
column = klass.columns_hash[attribute.to_s]
57-
value = column.limit ? value.to_s.mb_chars[0, column.limit] : value.to_s if column.text?
57+
value = column.limit ? value.to_s.mb_chars[0, column.limit] : value.to_s if value && column.text?
5858

5959
if !options[:case_sensitive] && value && column.text?
6060
# will use SQL LOWER function before comparison, unless it detects a case insensitive collation
@@ -81,7 +81,7 @@ module ClassMethods
8181
#
8282
# class Person < ActiveRecord::Base
8383
# validates_uniqueness_of :user_name, :scope => :account_id
84-
# end
84+
# end
8585
#
8686
# Or even multiple scope parameters. For example, making sure that a teacher can only be on the schedule once
8787
# per semester for a particular class.

activerecord/test/cases/validations/uniqueness_validation_test.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,4 +293,15 @@ def test_validate_straight_inheritance_uniqueness
293293
assert w6.errors[:city].any?, "Should have errors for city"
294294
assert_equal ["has already been taken"], w6.errors[:city], "Should have uniqueness message for city"
295295
end
296+
297+
def test_allow_nil_is_false
298+
Topic.validates_uniqueness_of(:title, :allow_nil => false)
299+
Topic.destroy_all
300+
301+
Topic.create!("title" => nil)
302+
topic = Topic.new("title" => nil)
303+
assert !topic.valid?, "topic should not be valid"
304+
assert topic.errors[:title].any?, "Should have errors for title"
305+
assert_equal ["has already been taken"], topic.errors[:title], "Should have uniqueness message for title"
306+
end
296307
end

0 commit comments

Comments
 (0)