From 380d18f0621c66a79445ebc6dcc0048fcc969911 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 9 May 2014 15:07:18 -0600 Subject: [PATCH] Revert 561ac3e2f0c9b717114f7452597062cf32584316 (Change `validate_uniqueness_of_matcher` to give non-null columns default values) This sounds good in theory, but we cannot choose default values that make sense in every single context. For instance, if the column only accepts certain values, then a default value must be chosen which is one of those values. There is no way we can know this. Instead of making `validate_uniqueness_of` magical, we should insist that people create a record that has the proper attributes with the proper values before using `validate_uniqueness_of`. --- NEWS.md | 13 +++++-- .../validate_uniqueness_of_matcher.rb | 13 +------ .../validate_uniqueness_of_matcher_spec.rb | 36 ------------------- 3 files changed, 12 insertions(+), 50 deletions(-) diff --git a/NEWS.md b/NEWS.md index fd699723..cdb57afc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,12 +1,21 @@ +# HEAD + +* Revert change to `validate_uniqueness_of` made in 2.6.0 so that it no longer + provides default values for non-primary, non-nullable columns. This approach + was causing test failures because it makes the assumption that none of these + columns allow only specific values, which is not true. If you get an error + from `validate_uniqueness_of`, your best bet continues to be creating a record + manually and calling `validate_uniqueness_of` on that instead. + # 2.6.1 -## Features +### Features * Teach `with_message` qualifier on `allow_value` to accept a hash of i18n interpolation values: `allow_value('foo').for(:attr).with_message(:greater_than, values: { count: 20 })`. -## Bug fixes +### Bug fixes * Revert changes to `validate_numericality_of` made in the last release, which made it so that comparison qualifiers specified on the validation are tested 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 83f7d01f..377000c8 100644 --- a/lib/shoulda/matchers/active_model/validate_uniqueness_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_uniqueness_of_matcher.rb @@ -115,11 +115,6 @@ module Shoulda # :nodoc: @subject.class.new.tap do |instance| instance.__send__("#{@attribute}=", value) - - other_non_nullable_columns.each do |non_nullable_column| - instance.__send__("#{non_nullable_column.name}=", correct_type_for_column(non_nullable_column)) - end - if has_secure_password? instance.password = 'password' instance.password_confirmation = 'password' @@ -203,7 +198,7 @@ module Shoulda # :nodoc: end def correct_type_for_column(column) - if column.type == :string || column.type == :binary + if column.type == :string '0' elsif column.type == :datetime DateTime.now @@ -225,12 +220,6 @@ module Shoulda # :nodoc: end value end - - def other_non_nullable_columns - @subject.class.columns.select do |column| - column.name != @attribute && !column.null && !column.primary - end - end end end end 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 c97a0c81..c3522769 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 @@ -383,42 +383,6 @@ describe Shoulda::Matchers::ActiveModel::ValidateUniquenessOfMatcher do end end - context "a model with non-nullable attribute" do - context "of type" do - [:string, :text, :integer, :float, :decimal, :datetime, :timestamp, :time, :date, :binary, :boolean].each do |type| - context type do - it "does not raise an error" do - model = define_model_with_non_nullable(type) - expect { expect(model).to matcher }.not_to raise_error - end - end - end - end - - context "that is a primary key" do - it "does not cause duplicate entry errors by re-using default values for primary keys" do - create_table :examples, id: false do |t| - t.string :attr - t.integer :non_nullable, primary: true - end - model_class = define_model(:example, attr: :string) do - validates_uniqueness_of :attr - end - model_1 = model_class.new - model_2 = model_class.new - expect(model_1).to matcher - expect { expect(model_2).to matcher }.not_to raise_error - end - end - - def define_model_with_non_nullable(type) - define_model(:example, attr: :string, non_nullable: { type: type, options: { null: false } }) do - attr_accessible :attr, :non_nullable - validates_uniqueness_of :attr - end.new - end - end - def case_sensitive_validation_with_existing_value(attr_type) model = define_model(:example, attr: attr_type) do attr_accessible :attr