diff --git a/NEWS.md b/NEWS.md index 6065ab3f..e7038fe6 100644 --- a/NEWS.md +++ b/NEWS.md @@ -29,6 +29,20 @@ (which is to permit this) is misleading, as the test that you write by using `allow_value` is different from the test that actually ends up getting run. +* Change behavior of `validate_uniqueness_of` when the matcher is not + qualified with any scopes, but your validation is. Previously the following + test would pass when it now fails: + + ``` ruby + class Post < ActiveRecord::Base + validate :slug, uniqueness: { scope: :user_id } + end + + describe Post do + it { should validate_uniqueness_of(:slug) } + end + ``` + ### Bug fixes * So far the tests for the gem have been running against only SQLite. Now they diff --git a/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb b/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb index 5a310858..6e5094f9 100644 --- a/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb +++ b/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb @@ -259,7 +259,8 @@ module Shoulda @expected_message ||= :taken @all_records = @subject.class.all - set_scoped_attributes && + scopes_match? && + set_scoped_attributes && validate_everything_except_duplicate_nils_or_blanks? && validate_case_sensitivity? && validate_after_scope_change? && @@ -271,6 +272,37 @@ module Shoulda private + def validation + @subject.class._validators[@attribute].detect do |validator| + validator.is_a?(::ActiveRecord::Validations::UniquenessValidator) + end + end + + def scopes_match? + expected_scopes = Array.wrap(@options[:scopes]) + + if validation + actual_scopes = Array.wrap(validation.options[:scope]) + else + actual_scopes = [] + end + + if expected_scopes == actual_scopes + true + else + @failure_message = "Expected validation to be scoped to " + + "#{expected_scopes}" + + if actual_scopes.present? + @failure_message << ", but it was scoped to #{actual_scopes}." + else + @failure_message << ", but it was not scoped to anything." + end + + false + end + end + def allows_nil? if @options[:allow_nil] ensure_nil_record_in_database diff --git a/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb index 03f6ecf8..537d0472 100644 --- a/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb @@ -113,11 +113,11 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo expect(record).not_to validate_uniqueness end - it 'accepts if the scope is unset beforehand' do + it 'rejects if the scope is unset beforehand' do record = build_record_validating_uniqueness( scopes: [ build_attribute(name: :scope, value: nil) ] ) - expect(record).to validate_uniqueness + expect(record).not_to validate_uniqueness end end @@ -130,8 +130,8 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo end end - define_method(:build_attribute) do |options| - options.merge( + define_method(:build_attribute) do |attribute_options| + attribute_options.merge( column_type: column_type, value_type: value_type, array: array