From 2065a654e6877b1ac7e815851bc7c4a634fa9644 Mon Sep 17 00:00:00 2001 From: "Ivan Valdes (@ivanvc)" Date: Fri, 17 May 2013 13:10:19 -0500 Subject: [PATCH] Fixes validate_uniqueness_of when scope is taken Re-implement how to get previous value in order to test validate_uniquenes_of matcher, after the scope changes. This way, new value should not be taken by a previous record. Fixes #207. --- NEWS.md | 3 ++ .../validate_uniqueness_of_matcher.rb | 6 +-- .../validate_uniqueness_of_matcher_spec.rb | 37 ++++++++++++++++++- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/NEWS.md b/NEWS.md index bcfb96b5..0d767a9c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,9 @@ * Fix `have_and_belong_to_many` matcher issue for Rails 4. +* Fix `validate_uniqueness_of.scoped_to` issue when the scoped field is already + taken (#207). + # v 2.1.0 * Add missing `failure_message_for_should_not` implementations to diff --git a/lib/shoulda/matchers/active_model/validate_uniqueness_of_matcher.rb b/lib/shoulda/matchers/active_model/validate_uniqueness_of_matcher.rb index 4549e33f..0dbf5297 100644 --- a/lib/shoulda/matchers/active_model/validate_uniqueness_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_uniqueness_of_matcher.rb @@ -156,15 +156,13 @@ module Shoulda # :nodoc: @existing_record = create_record_in_database end - # TODO: There is a chance that we could change the scoped field - # to a value that's already taken. An alternative implementation - # could actually find all values for scope and create a unique def validate_after_scope_change? if @options[:scopes].blank? true else + all_records = @subject.class.all @options[:scopes].all? do |scope| - previous_value = existing_record.send(scope) + previous_value = all_records.map(&scope).max # Assume the scope is a foreign key if the field is nil previous_value ||= correct_type_for_column(@subject.class.columns_hash[scope.to_s]) diff --git a/spec/shoulda/matchers/active_model/validate_uniqueness_of_matcher_spec.rb b/spec/shoulda/matchers/active_model/validate_uniqueness_of_matcher_spec.rb index 52cc0b57..089187af 100644 --- a/spec/shoulda/matchers/active_model/validate_uniqueness_of_matcher_spec.rb +++ b/spec/shoulda/matchers/active_model/validate_uniqueness_of_matcher_spec.rb @@ -118,6 +118,13 @@ describe Shoulda::Matchers::ActiveModel::ValidateUniquenessOfMatcher do should matcher.scoped_to(:scope1) end + context 'with an existing record that conflicts with scope.next' do + it 'accepts' do + validating_scoped_uniqueness_with_conflicting_next(:scope1, :date, :scope1 => Date.today). + should matcher.scoped_to(:scope1) + end + end + context 'when too narrow of a scope is specified' do it 'rejects' do validating_scoped_uniqueness([:scope1, :scope2], :date, :scope1 => Date.today, :scope2 => Date.today). @@ -132,6 +139,13 @@ describe Shoulda::Matchers::ActiveModel::ValidateUniquenessOfMatcher do should matcher.scoped_to(:scope1) end + context 'with an existing record that conflicts with scope.next' do + it 'accepts' do + validating_scoped_uniqueness_with_conflicting_next(:scope1, :datetime, :scope1 => DateTime.now). + should matcher.scoped_to(:scope1) + end + end + context 'with a nil value' do it 'accepts' do validating_scoped_uniqueness([:scope1], :datetime, :scope1 => nil). @@ -145,11 +159,22 @@ describe Shoulda::Matchers::ActiveModel::ValidateUniquenessOfMatcher do should_not matcher.scoped_to(:scope1, :scope2, :other) end end + + context 'with an existing record that conflicts with scope.next' do + it 'accepts' do + validating_scoped_uniqueness_with_conflicting_next(:scope1, :scope1 => 1). + should matcher.scoped_to(:scope1) + end + end end def create_existing_record(attributes = {}) + @existing ||= create_record(attributes) + end + + def create_record(attributes = {}) default_attributes = {:attr => 'value', :scope1 => 1, :scope2 => 2, :other => 3} - @existing ||= Example.create!(default_attributes.merge(attributes)) + Example.create!(default_attributes.merge(attributes)) end def define_scoped_model(scope, scope_attr_type = :integer) @@ -166,6 +191,16 @@ describe Shoulda::Matchers::ActiveModel::ValidateUniquenessOfMatcher do create_existing_record(attributes) model end + + def validating_scoped_uniqueness_with_conflicting_next(*args) + attributes = args.extract_options! + model = define_scoped_model(*args).new + 2.times do + attributes[:scope1] = attributes[:scope1].next + create_record(attributes) + end + model + end end context 'a model with a case-sensitive uniqueness validation on a string attribute and an existing record' do