From cf1ea9ab86ca8633cde51f8455e2d3b4af865f2f Mon Sep 17 00:00:00 2001 From: "Carlos A. da Silva" Date: Wed, 7 Oct 2009 22:33:45 -0300 Subject: [PATCH] Cleaning up recoverable methods, changing to use a hash of options instead of default parameters. --- app/controllers/passwords_controller.rb | 7 +++---- lib/devise/models/recoverable.rb | 14 ++++++++------ test/models/authenticable_test.rb | 2 +- test/models/recoverable_test.rb | 22 +++++++++++++--------- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index 1c3137c2..aa2aa0de 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -5,8 +5,8 @@ class PasswordsController < ApplicationController end def create - @password = User.find_and_send_reset_password_instructions(params[:password][:email]) - if !@password.new_record? + @password = User.send_reset_password_instructions(params[:password]) + if @password.errors.empty? flash[:notice] = 'You will receive an email with instructions about how to reset your password in a few minutes.' redirect_to new_session_path else @@ -20,8 +20,7 @@ class PasswordsController < ApplicationController end def update - @password = User.find_and_reset_password(params[:password][:perishable_token], - params[:password][:password], params[:password][:password_confirmation]) + @password = User.reset_password(params[:password]) if @password.errors.empty? flash[:notice] = 'Your password was changed successfully.' redirect_to new_session_path diff --git a/lib/devise/models/recoverable.rb b/lib/devise/models/recoverable.rb index 09844e3a..6c4da743 100644 --- a/lib/devise/models/recoverable.rb +++ b/lib/devise/models/recoverable.rb @@ -34,9 +34,10 @@ module Devise # Attempt to find a user by it's email. If a record is found, send new # password instructions to it. If not user is found, returns a new user # with an email not found error. + # Options must contain the user email # - def find_and_send_reset_password_instructions(email) - recoverable = find_or_initialize_by_email(email) + def send_reset_password_instructions(options={}) + recoverable = find_or_initialize_by_email(options[:email]) unless recoverable.new_record? recoverable.send_reset_password_instructions else @@ -48,12 +49,13 @@ module Devise # Attempt to find a user by it's perishable_token to reset it's password. # If a user is found, reset it's password and automatically try saving the # record. If not user is found, returns a new user containing an error - # in perishable_token attribute + # in perishable_token attribute. + # Options must contain perishable_token, password and confirmation # - def find_and_reset_password(perishable_token, password=nil, password_confirmation=nil) - recoverable = find_or_initialize_by_perishable_token(perishable_token) + def reset_password(options={}) + recoverable = find_or_initialize_by_perishable_token(options[:perishable_token]) unless recoverable.new_record? - recoverable.reset_password!(password, password_confirmation) + recoverable.reset_password!(options[:password], options[:password_confirmation]) else recoverable.errors.add(:perishable_token, :invalid, :default => "invalid confirmation") end diff --git a/test/models/authenticable_test.rb b/test/models/authenticable_test.rb index 5f71248a..a14de8b6 100644 --- a/test/models/authenticable_test.rb +++ b/test/models/authenticable_test.rb @@ -104,7 +104,7 @@ class AuthenticableTest < ActiveSupport::TestCase end test 'should authenticate a valid user with email and password and return it' do - user = User.create!(valid_attributes) + user = create_user User.any_instance.stubs(:confirmed?).returns(true) authenticated_user = User.authenticate(user.email, user.password) assert_equal authenticated_user, user diff --git a/test/models/recoverable_test.rb b/test/models/recoverable_test.rb index 14dd5da0..1c004529 100644 --- a/test/models/recoverable_test.rb +++ b/test/models/recoverable_test.rb @@ -32,56 +32,60 @@ class RecoverableTest < ActiveSupport::TestCase end test 'should find a user to send instructions by email' do - reset_password_user = User.find_and_send_reset_password_instructions(@user.email) + reset_password_user = User.send_reset_password_instructions(:email => @user.email) assert_not_nil reset_password_user assert_equal reset_password_user, @user end test 'should return a new user if no email was found' do - reset_password_user = User.find_and_send_reset_password_instructions("invalid@email.com") + reset_password_user = User.send_reset_password_instructions(:email => "invalid@email.com") assert_not_nil reset_password_user assert reset_password_user.new_record? end test 'should add error to new user email if no email was found' do - reset_password_user = User.find_and_send_reset_password_instructions("invalid@email.com") + reset_password_user = User.send_reset_password_instructions(:email => "invalid@email.com") assert reset_password_user.errors[:email] assert_equal 'not found', reset_password_user.errors[:email] end test 'should reset perishable token before send the reset instructions email' do token = @user.perishable_token - reset_password_user = User.find_and_send_reset_password_instructions(@user.email) + reset_password_user = User.send_reset_password_instructions(:email => @user.email) assert_not_equal token, @user.reload.perishable_token end test 'should send email instructions to the user reset it\'s password' do assert_email_sent do - User.find_and_send_reset_password_instructions(@user.email) + User.send_reset_password_instructions(:email => @user.email) end end test 'should find a user to reset it\'s password based on perishable_token' do - reset_password_user = User.find_and_reset_password(@user.perishable_token) + reset_password_user = User.reset_password(:perishable_token => @user.perishable_token) assert_not_nil reset_password_user assert_equal reset_password_user, @user end test 'should return a new user when trying to reset it\'s password if no perishable_token is found' do - reset_password_user = User.find_and_reset_password('invalid_token') + reset_password_user = User.reset_password(:perishable_token => 'invalid_token') assert_not_nil reset_password_user assert reset_password_user.new_record? end test 'should add error to new user email if no perishable token was found' do - reset_password_user = User.find_and_reset_password("invalid_token") + reset_password_user = User.reset_password(:perishable_token => "invalid_token") assert reset_password_user.errors[:perishable_token] assert_equal 'invalid confirmation', reset_password_user.errors[:perishable_token] end test 'should reset successfully user password given the new password and confirmation' do old_password = @user.password - reset_password_user = User.find_and_reset_password(@user.perishable_token, 'new_password', 'new_password') + reset_password_user = User.reset_password( + :perishable_token => @user.perishable_token, + :password => 'new_password', + :password_confirmation => 'new_password' + ) @user.reload assert_not @user.valid_password?(old_password) assert @user.valid_password?('new_password')