1
\$\begingroup\$

I have to update user email and password if they are present in the params, I have other user information also which can be updated.

this is the method I have written, but it doesn't seem like a good method

def update
    if user_params[:current_email].present?
      if @current_user.has_valid_email?(user_params[:current_email])
        @current_user.update(email: user_params[:new_email])
      else
        render json: {errors: ["Current Email did not match!"]}, status: :unprocessable_entity and return
      end
    end
    if user_params[:current_password].present?
       if @current_user.has_valid_password?(user_params[:current_password])
        @current_user.update(password: user_params[:new_password])
      else
        render json: {errors: ["Current Password did not match!"]}, status: :unprocessable_entity and return
      end
    end
    if @current_user.update(sanitized_params)
      @web_user = @current_user
      render :show
    else
      render json: {errors: @current_user.errors.full_messages}, status: :unprocessable_entity
    end
  end




private

  def sanitized_params
    user_params.slice!(:current_email,:new_email,:current_password,:new_password)
  end

  def user_params
    params.permit(:current_email, :new_email, :current_password, :new_password, :password_confirmation, :reminders_frequency,
                  :coaching_style, :coaching_style_status,:suggestion_preference,:language,:region)
  end
\$\endgroup\$
1
\$\begingroup\$

A few obvious things would be:

1) You shouldn't include current_email and current_password in the sanitized_params since you don't want to update these only validate them.

2) Similarly for new_email and new_password, you have aleady updated these manually

3) So really you aren't updating anything else currently.

4) If your first two tests fail you render a json error but don't stop processing so the last if clause will still execute and you will get a double render error. You should probably test all your conditions before executing any code.

I would consider moving the update logic into methods on the User model (update_email, #update_password that do the validation for you.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.