diff --git a/NEWS.md b/NEWS.md index 3ea026d63..d13926bf0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -23,6 +23,15 @@ * *Commits: [45de869]* * *Fixes: [#880], [#884], [#885]* +* Fix an issue with `validate_uniqueness_of` + `scoped_to` when used against a + model where the attribute has multiple uniqueness validations and each + validation has a different set of scopes. In this case, a test written for the + first validation (and its scopes) would pass, but tests for the other + validations (and their scopes) would not, as the matcher only considered the + first set of scopes as the *actual* set of scopes. + + * *References: [#830]* + ### Improvements * Update `validate_uniqueness_of` so that if an existing record fails to be @@ -37,6 +46,7 @@ [#884]: https://github.com/thoughtbot/shoulda-matchers/issues/884 [#885]: https://github.com/thoughtbot/shoulda-matchers/issues/885 [78ccfc5]: https://github.com/thoughtbot/shoulda-matchers/commit/78ccfc50b52fa686c109d614df66744b0da65380 +[#830]: https://github.com/thoughtbot/shoulda-matchers/issues/830 # 3.1.0 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 c197f73a9..f905c576e 100644 --- a/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb +++ b/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb @@ -326,7 +326,7 @@ def matches?(given_record) validate_attribute_present_on_model? && validate_scopes_present_on_model? && - scopes_match? && + validate_scopes_match? && validate_two_records_with_same_non_blank_value_cannot_coexist? && validate_case_sensitivity? && validate_after_scope_change? && @@ -380,14 +380,14 @@ def description_for_case_sensitive_qualifier end end - def validation - model._validators[@attribute].detect do |validator| + def validations + model._validators[@attribute].select do |validator| validator.is_a?(::ActiveRecord::Validations::UniquenessValidator) end end - def scopes_match? - if expected_scopes == actual_scopes + def validate_scopes_match? + if scopes_match? true else @failure_reason = 'Expected the validation' @@ -398,7 +398,7 @@ def scopes_match? @failure_reason << " to be scoped to #{inspected_expected_scopes}" end - if actual_scopes.empty? + if actual_sets_of_scopes.empty? @failure_reason << ', but it was not scoped to anything.' else @failure_reason << ', but it was scoped to ' @@ -409,24 +409,42 @@ def scopes_match? end end - def expected_scopes - Array.wrap(@options[:scopes]) + def scopes_match? + actual_sets_of_scopes.empty? && expected_scopes.empty? || + actual_sets_of_scopes.any? { |scopes| scopes == expected_scopes } end def inspected_expected_scopes expected_scopes.map(&:inspect).to_sentence end - def actual_scopes - if validation - Array.wrap(validation.options[:scope]) + def inspected_actual_scopes + inspected_actual_sets_of_scopes.to_sentence( + words_connector: " or ", + last_word_connector: ", or" + ) + end + + def inspected_actual_sets_of_scopes + inspected_sets_of_scopes = actual_sets_of_scopes.map do |scopes| + scopes.map(&:inspect) + end + + if inspected_sets_of_scopes.many? + inspected_sets_of_scopes.map { |x| "(#{x.to_sentence})" } else - [] + inspected_sets_of_scopes.map(&:to_sentence) end end - def inspected_actual_scopes - actual_scopes.map(&:inspect).to_sentence + def expected_scopes + Array.wrap(@options[:scopes]) + end + + def actual_sets_of_scopes + validations.map do |validation| + Array.wrap(validation.options[:scope]) + end.reject(&:empty?) end def allows_nil? 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 191b44afa..114537bcf 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 @@ -227,6 +227,94 @@ end end + context 'when there is more than one validation on the same attribute with different scopes' do + context 'when a record exists beforehand, where all scopes are set' do + if column_type != :boolean + context 'when each validation has the same (default) message' do + it 'accepts' do + pending 'this needs another qualifier to properly fix' + + model = define_model( + 'Example', + attribute_name => :string, + scope1: column_type, + scope2: column_type + ) do |m| + m.validates_uniqueness_of(attribute_name, scope: [:scope1]) + m.validates_uniqueness_of(attribute_name, scope: [:scope2]) + end + + model.create!( + attribute_name => dummy_value_for(:string), + scope1: dummy_value_for(column_type), + scope2: dummy_value_for(column_type) + ) + + expect(model.new).to validate_uniqueness.scoped_to(:scope1) + expect(model.new).to validate_uniqueness.scoped_to(:scope2) + end + end + end + + context 'when each validation has a different message' do + it 'accepts' do + model = define_model( + 'Example', + attribute_name => :string, + scope1: column_type, + scope2: column_type + ) do |m| + m.validates_uniqueness_of( + attribute_name, + scope: [:scope1], + message: 'first message' + ) + m.validates_uniqueness_of( + attribute_name, + scope: [:scope2], + message: 'second message' + ) + end + + model.create!( + attribute_name => dummy_value_for(:string), + scope1: dummy_value_for(column_type), + scope2: dummy_value_for(column_type) + ) + + expect(model.new). + to validate_uniqueness. + scoped_to(:scope1). + with_message('first message') + + expect(model.new). + to validate_uniqueness. + scoped_to(:scope2). + with_message('second message') + end + end + end + + context 'when no record exists beforehand' do + it 'accepts' do + pending 'this needs another qualifier to properly fix' + + model = define_model( + 'Example', + attribute_name => :string, + scope1: column_type, + scope2: column_type + ) do |m| + m.validates_uniqueness_of(attribute_name, scope: [:scope1]) + m.validates_uniqueness_of(attribute_name, scope: [:scope2]) + end + + expect(model.new).to validate_uniqueness.scoped_to(:scope1) + expect(model.new).to validate_uniqueness.scoped_to(:scope2) + end + end + end + define_method(:build_attribute) do |attribute_options| attribute_options.deep_merge( column_type: column_type,