Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

refactor Users#reset_password_create action to reduce complexity

Extract password reset logic in PasswordService class.
  • Loading branch information...
commit 1ad327ef02ac936b005edee79191562eae491f4b 1 parent f7d0306
@hindenbug hindenbug authored
View
51 app/controllers/users_controller.rb
@@ -260,49 +260,22 @@ def reset_password_new
# Submitted passwords are checked for length and confirmation. Once the
# password has been reset the user is redirected to /
def reset_password_create
- user = nil
+ user = User.first(:perishable_token => params[:token]) if params[:token]
+ user = nil if user && user.token_expired?
- if params[:token]
- user = User.first(:perishable_token => params[:token])
- if user and user.token_expired?
- user = nil
- end
- end
-
- unless user.nil?
- # XXX: yes, this is a code smell
-
- if params[:password].size == 0
- flash[:error] = "Password must be present"
- redirect_to reset_password_path(params[:token])
- return
- end
-
- if params[:password] != params[:password_confirm]
- flash[:error] = "Passwords do not match"
- redirect_to reset_password_path(params[:token])
- return
- end
-
- # TODO: This may be unreachable, since we don't allow password reset
- # without an email... look into this and remove this code if so.
- if user.email.nil?
- if params[:email].empty?
- flash[:error] = "Email must be provided"
- redirect_to reset_password_path(params[:token])
- return
- else
- user.email = params[:email]
- end
- end
+ redirect_to forgot_password_path unless user
+ password_service = PasswordService.new(user, params)
- user.password = params[:password]
- user.save
- flash[:notice] = "Password successfully set"
- redirect_to root_path
+ if password_service.invalid?
+ flash[:error] = password_service.message
+ url = reset_password_path(params[:token])
else
- redirect_to forgot_password_path
+ password_service.reset_password
+ flash[:notice] = "Password successfully set"
+ url = root_path
end
+
+ redirect_to url
end
# Public reset password page, accessible via a valid token. Tokens are only
View
47 app/services/password_service.rb
@@ -0,0 +1,47 @@
+class PasswordService
+
+ attr_accessor :message
+
+ def initialize(user, options={})
+ @user = user
+ @password = options[:password]
+ @confirm_password = options[:password_confirm]
+ @email = options[:email]
+ @message = fetch_message
+ end
+
+ def invalid?
+ password_missing? || password_mismatch? || email_missing?
+ end
+
+ def reset_password
+ @user.password = @password
+ @user.save
+ end
+
+ private
+
+ def password_missing?
+ @password.blank?
+ end
+
+ def password_mismatch?
+ @password != @confirm_password
+ end
+
+ def email_missing?
+ @user.email ||= @email
+ @user.email.blank?
+ end
+
+ def fetch_message
+ if password_missing?
+ "Password must be present"
+ elsif password_mismatch?
+ "Passwords do not match"
+ elsif email_missing?
+ "Email must be provided"
+ end
+ end
+
+end
View
108 test/services/password_service_test.rb
@@ -0,0 +1,108 @@
+require_relative '../test_helper'
+
+describe PasswordService do
+ include TestHelper
+
+ let(:user) { Fabricate(:user) }
+
+ describe "#invalid?" do
+ describe "when password_missing?" do
+ describe "when password is not present" do
+ let(:options) { { password: "" } }
+ let(:password_service) { PasswordService.new(user, options) }
+
+ it "should be invalid?" do
+ assert_equal true, password_service.invalid?
+ end
+ end
+
+ describe "when password is present" do
+ let(:options) { { password: "password" } }
+ let(:password_service) { PasswordService.new(user, options) }
+
+ it "should be valid" do
+ password_service.stubs(:password_mismatch?).returns false
+ password_service.stubs(:email_missing?).returns false
+ assert_equal false, password_service.invalid?
+ end
+ end
+ end
+
+ describe "when password_mismatch?" do
+ describe "when password and confirm pasword don't match" do
+ let(:options) { { password: "password", password_confirm: "somepassword" } }
+ let(:password_service) { PasswordService.new(user, options) }
+
+ it "should not be valid" do
+ assert_equal true, password_service.invalid?
+ end
+ end
+
+ describe "when password and confirm pasword match" do
+ let(:options) { { password: "password", password_confirm: "password" } }
+ let(:password_service) { PasswordService.new(user, options) }
+
+ it "should be valid" do
+ password_service.stubs(:password_missing?).returns false
+ password_service.stubs(:email_missing?).returns false
+ assert_equal false, password_service.invalid?
+ end
+ end
+ end
+
+ describe "when email_missing?" do
+ describe "when user email not present" do
+ let(:user) { Fabricate(:user, email: nil) }
+
+ describe "and email not present in params" do
+ let(:password_service) { PasswordService.new(user) }
+
+ it "should return true" do
+ assert_equal true, password_service.invalid?
+ end
+ end
+
+ describe "and email is present in params" do
+ let(:password_service) { PasswordService.new(user, { email: "some@email.com"}) }
+
+ it "should return false" do
+ password_service.stubs(:password_missing?).returns false
+ password_service.stubs(:password_mismatch?).returns false
+
+ assert_equal false, password_service.invalid?
+ end
+
+ it "should set the params email as the use email" do
+ password_service.stubs(:password_missing?).returns false
+ password_service.stubs(:password_mismatch?).returns false
+
+ assert_equal false, password_service.invalid?
+ end
+ end
+ end
+
+ describe "when user email is present" do
+ let(:user) { Fabricate(:user, email: "some@example.com") }
+ let(:password_service) { PasswordService.new(user) }
+
+ it "should NOT say email is missing" do
+ password_service.stubs(:password_missing?).returns false
+ password_service.stubs(:password_mismatch?).returns false
+
+ assert_equal false, password_service.invalid?
+ end
+ end
+ end
+ end
+
+ describe "#reset_password" do
+ let(:user) { Fabricate(:user, email: "some@example.com") }
+ let(:password_service) { PasswordService.new(user, password: "password") }
+
+ it "should reset the password successfully" do
+ assert_equal true, password_service.reset_password
+ end
+
+ end
+
+end
Please sign in to comment.
Something went wrong with that request. Please try again.