From b501ee47fa3f877f8b8028e732f8ef8a22cc75fb Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Mon, 4 Mar 2013 18:50:56 +0100 Subject: `validates_confirmation_of` does not override writer methods. --- activemodel/CHANGELOG.md | 15 +++++++++++ .../lib/active_model/validations/confirmation.rb | 6 ++++- .../validations/confirmation_validation_test.rb | 31 ++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) (limited to 'activemodel') diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 1fe6dbd4d9..e518334d5d 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,5 +1,20 @@ ## Rails 4.0.0 (unreleased) ## +* `validates_confirmation_of` does not override writer methods for + the confirmation attribute if no reader is defined. + + Example: + + class Blog + def title=(new_title) + @title = new_title.downcase + end + + # previously this would override the setter above. + validates_confirmation_of :title + end + + *Yves Senn* ## Rails 4.0.0.beta1 (February 25, 2013) ## diff --git a/activemodel/lib/active_model/validations/confirmation.rb b/activemodel/lib/active_model/validations/confirmation.rb index 3a3abce364..d14fb4dc53 100644 --- a/activemodel/lib/active_model/validations/confirmation.rb +++ b/activemodel/lib/active_model/validations/confirmation.rb @@ -10,9 +10,13 @@ module ActiveModel end def setup(klass) - klass.send(:attr_accessor, *attributes.map do |attribute| + klass.send(:attr_reader, *attributes.map do |attribute| :"#{attribute}_confirmation" unless klass.method_defined?(:"#{attribute}_confirmation") end.compact) + + klass.send(:attr_writer, *attributes.map do |attribute| + :"#{attribute}_confirmation" unless klass.method_defined?(:"#{attribute}_confirmation=") + end.compact) end end diff --git a/activemodel/test/cases/validations/confirmation_validation_test.rb b/activemodel/test/cases/validations/confirmation_validation_test.rb index f7556a249f..814eec3f59 100644 --- a/activemodel/test/cases/validations/confirmation_validation_test.rb +++ b/activemodel/test/cases/validations/confirmation_validation_test.rb @@ -71,4 +71,35 @@ class ConfirmationValidationTest < ActiveModel::TestCase I18n.backend = @old_backend end + test "does not override confirmation reader if present" do + klass = Class.new do + include ActiveModel::Validations + + def title_confirmation + "expected title" + end + + validates_confirmation_of :title + end + + assert_equal "expected title", klass.new.title_confirmation, + "confirmation validation should not override the reader" + end + + test "does not override confirmation writer if present" do + klass = Class.new do + include ActiveModel::Validations + + def title_confirmation=(value) + @title_confirmation = "expected title" + end + + validates_confirmation_of :title + end + + model = klass.new + model.title_confirmation = "new title" + assert_equal "expected title", model.title_confirmation, + "confirmation validation should not override the writer" + end end -- cgit v1.2.3 From 8c1687bbf8dd518d64fc7180b33c1cb57f29a69a Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Mon, 4 Mar 2013 18:56:05 +0100 Subject: `has_secure_password` is not invalid when assigning empty Strings. Closes #9535. With 692b3b6 the `password=` setter does no longer set blank passwords. This triggered validation errors when assigning empty Strings to `password` and `password_confirmation`. This patch only sets the confirmation if it is not `blank?`. --- activemodel/CHANGELOG.md | 10 ++++++++++ activemodel/lib/active_model/secure_password.rb | 10 ++++++++-- activemodel/test/cases/secure_password_test.rb | 6 ++++++ 3 files changed, 24 insertions(+), 2 deletions(-) (limited to 'activemodel') diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index e518334d5d..8c54ec3d45 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,5 +1,15 @@ ## Rails 4.0.0 (unreleased) ## +* `has_secure_password` does not fail the confirmation validation + when assigning empty String to `password` and `password_confirmation`. + + Example: + + # given User has_secure_password. + @user.password = "" + @user.password_confirmation = "" + @user.valid?(:update) # used to be false + * `validates_confirmation_of` does not override writer methods for the confirmation attribute if no reader is defined. diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 6644b60609..9324a1ad0a 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -48,6 +48,8 @@ module ActiveModel attr_reader :password + include InstanceMethodsOnActivation + if options.fetch(:validations, true) validates_confirmation_of :password validates_presence_of :password, :on => :create @@ -55,8 +57,6 @@ module ActiveModel before_create { raise "Password digest missing on new record" if password_digest.blank? } end - include InstanceMethodsOnActivation - if respond_to?(:attributes_protected_by_default) def self.attributes_protected_by_default #:nodoc: super + ['password_digest'] @@ -99,6 +99,12 @@ module ActiveModel self.password_digest = BCrypt::Password.create(unencrypted_password, cost: cost) end end + + def password_confirmation=(unencrypted_password) + unless unencrypted_password.blank? + @password_confirmation = unencrypted_password + end + end end end end diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb index 7783bb25d5..02cd3b8a93 100644 --- a/activemodel/test/cases/secure_password_test.rb +++ b/activemodel/test/cases/secure_password_test.rb @@ -88,4 +88,10 @@ class SecurePasswordTest < ActiveModel::TestCase @user.password = "secret" assert_equal BCrypt::Engine::MIN_COST, @user.password_digest.cost end + + test "blank password_confirmation does not result in a confirmation error" do + @user.password = "" + @user.password_confirmation = "" + assert @user.valid?(:update), "user should be valid" + end end -- cgit v1.2.3