Sajan
Sajan

Reputation: 1923

confusing validation error in rails model

Below is my model and controller from which i have filtered unneccessary lines.

class User < ActiveRecord::Base
validates :password, presence: true, on: :create
validates :password_confirmation, presence: true, if: "password.present?"
validates :password, confirmation: true, 
  length: { in: 6..20}, if: "password.present?"
end

and controller-

class UsersController < ApplicationController
  def update
    @user = User.find(params[:id])
    if params[:password].present?
      final_params = get_params
    else
      final_params = get_params.delete_if { |k,v| 
        k == "password" or k == "password_confirmation"
      }
    end

    if @user.update(final_params)
      redirect_to @user
    else
      render 'edit'
    end
  end

  private
  def get_params
    params.require(:user).permit(:first_name, :last_name, :email, :date_of_birth, 
    :password,:password_confirmation, :mobile, :country, :state, :city, :pincode)
  end
end

the problem is when updating a data, it shows a validation error i.e password confirmation can not be blank. even if I enter something to that field and submit. and to find error i tried replacing "password.present?" from password confirmation validation with "password.exists?" and it showed exception that exists is not a valid method for "123456 : string" . 123456 is the current password in DB. why is it checking password against db ? and please help me to solve this.

Upvotes: 0

Views: 75

Answers (2)

Ajay
Ajay

Reputation: 4251

It's never a good idea to chunk down the incoming parameters in the controller.

Rather, putting proper validations in the model is a good idea ! hence cleaned up your controller.

Check below code:

class UsersController < ApplicationController
  def update
    @user = User.find(params[:id])
    redirect_to @user and return  if @user.update_attributes(get_params)
    # render will not be executed if the user is redirected & returned
    render :edit
  end

  private
  def get_params
    params.require(:user).permit(:first_name, :last_name, :email, :date_of_birth, 
    :password, :password_confirmation, :mobile, :country :state, :city, :pincode)
  end
end

modified model:

class User < ActiveRecord::Base
  validates :password, presence: true, on: :create
  # above validation will be effective only for during new record creation.

  # below 2 validations will be cheked only if password is present in the params list.
  validates :password, confirmation: true, 
    length: { in: 6..20 }, if: validate_password? 
  validates :password_confirmation, presence: true, if: validate_password? 

    private
    def validate_password?
      password.present?
    end
end

if still this does not help, then try to debug the self object in the validate_password? method. use raise self.inspect in the validation method to verify the incoming parameters. That way you can track where you are going wrong.

Upvotes: 1

Taryn East
Taryn East

Reputation: 27747

if params[:password].present?
    final_params = get_params
else
    final_params = get_params.delete_if { |k,v| k == "password" or k == "password_confirmation"}
end

Your problem is the first line here... your params are params[:user][:password] not params[:password] (you can see that in your get_params method)

So always your code is going to run the section that removes the password/confirmation

Also:

validates :password_confirmation, presence: true, if: "password.present?"

using a string of ruby in the validation is not generally considered good practice. How about adding a method like so:

validates :password_confirmation, presence: true, if: :confirmation_needed?
def confirmation_needed?
  password.present?
end

Finally, you also need to not check the length of password_confirmation if it hasn't actually been entered:

validates :password, confirmation: true, length: { in: 6..20}, 
          allow_blank: true, if: :confirmation_needed?

Upvotes: 1

Related Questions