ga-wdi-boston / full-stack-project

Other
8 stars 64 forks source link

Password can't be blank, but I already entered one...? #661

Closed laurpaik-zz closed 7 years ago

laurpaik-zz commented 7 years ago

Hi, I am trying to move my create athlete into my user controller to get the athlete and signUp to happen at the same time, but I'm having a hard time testing it out. My user controller:

  def signup
    user = User.create(user_creds[:credentials])
    if user.valid?
      @athlete = user.create_athlete(user_creds[:athlete])
      render json: user, status: :created
    else
      render json: user.errors, status: :bad_request
    end
  end
  def user_creds
    params.require(:credentials)
          .permit(:email, :password, :password_confirmation)
    params.require(:athlete)
          .permit(:given_name, :surname, :date_of_birth)
  end

User model and athlete model:

class User < ApplicationRecord
  include Authentication
  has_many :examples
  has_one :athlete
  has_many :logs, through: :athlete
end

class User < ApplicationRecord
  include Authentication
  has_many :examples
  has_one :athlete
  has_many :logs, through: :athlete
end

sign up script:

API="${API_ORIGIN:-http://localhost:4741}"
URL_PATH="/sign-up"
curl "${API}${URL_PATH}" \
  --include \
  --request POST \
  --header "Content-Type: application/json" \
  --data '{
    "credentials": {
      "email": "'"${EMAIL}"'",
      "password": "'"${PASSWORD}"'",
      "password_confirmation": "'"${PASSWORD}"'"
    },
    "athlete": {
      "given_name": "'"${GIVEN_NAME}"'",
      "surname": "'"${SURNAME}"'",
      "date_of_birth": "'"${DOB}"'"
    }
  }'

echo

Server error:

Started POST "/sign-up" for 127.0.0.1 at 2017-02-21 13:54:40 -0500
Processing by UsersController#signup as */*
  Parameters: {"credentials"=>{"email"=>"lexie@lexie.com", "password"=>"[FILTERED]", "password_confirmation"=>"[FILTERED]"}, "athlete"=>{"given_name"=>"Nayoon", "surname"=>"Bae", "date_of_birth"=>"2001-12-28"}}
   (0.1ms)  BEGIN
  User Exists (0.3ms)  SELECT  1 AS one FROM "users" WHERE "users"."email" IS NULL LIMIT $1  [["LIMIT", 1]]
   (0.1ms)  ROLLBACK
  User Exists (0.2ms)  SELECT  1 AS one FROM "users" WHERE "users"."email" IS NULL LIMIT $1  [["LIMIT", 1]]
[active_model_serializers] Rendered ActiveModel::Serializer::Null with ActiveModel::Errors (0.1ms)
Completed 400 Bad Request in 19ms (Views: 5.6ms | ActiveRecord: 0.7ms)

other error:

$ EMAIL='lexie@lexie.com' PASSWORD='abc123' GIVEN_NAME='Nayoon' SURNAME='Bae' DOB='2001-12-28' sh scripts/sign-up.sh 
HTTP/1.1 400 Bad Request
X-Frame-Options: SAMEORIGIN
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
Content-Type: application/json; charset=utf-8
Cache-Control: no-cache
X-Request-Id: 2e2402db-6847-4ccf-a0e2-c2b8c0288cd1
X-Runtime: 0.042415
Vary: Origin
Transfer-Encoding: chunked

{"password":["can't be blank"]}
bernardlee commented 7 years ago

It looks like you accidentally pasted the same code twice when you meant to paste your Athlete model's code.

laurpaik-zz commented 7 years ago

Ah, sorry

class Athlete < ApplicationRecord
  has_many :workouts, through: :logs
  has_many :logs
  belongs_to :user

  validates :given_name, presence: true
  validates :surname, presence: true
  validates :date_of_birth, presence: true
end
laurpaik-zz commented 7 years ago

Also, if I do this:

  def signup
    user = User.create(user_creds)
    if user.valid? # user.create before the function ends
      @athlete = user.build_athlete(user_creds[:athlete])
      render json: user, status: :created
    else
      render json: user.errors, status: :bad_request
    end
  end
  def user_creds
    params.require(:credentials)
          .permit(:email, :password, :password_confirmation,
                  athlete: [:given_name, :surname, :date_of_birth])
  end
API="${API_ORIGIN:-http://localhost:4741}"
URL_PATH="/sign-up"
curl "${API}${URL_PATH}" \
  --include \
  --request POST \
  --header "Content-Type: application/json" \
  --data '{
    "credentials": {
      "email": "'"${EMAIL}"'",
      "password": "'"${PASSWORD}"'",
      "password_confirmation": "'"${PASSWORD}"'",
      "athlete": {
        "given_name": "'"${GIVEN_NAME}"'",
        "surname": "'"${SURNAME}"'",
        "date_of_birth": "'"${DOB}"'"
      }
    }
  }'

echo

I get:

Started POST "/sign-up" for 127.0.0.1 at 2017-02-21 14:05:19 -0500
Processing by UsersController#signup as */*
  Parameters: {"credentials"=>{"email"=>"lexie@lexie.com", "password"=>"[FILTERED]", "password_confirmation"=>"[FILTERED]", "athlete"=>{"given_name"=>"Nayoon", "surname"=>"Bae", "date_of_birth"=>"2001-12-28"}}}
Completed 500 Internal Server Error in 80ms (ActiveRecord: 0.0ms)

ActiveRecord::AssociationTypeMismatch (Athlete(#70330229857380) expected, got ActiveSupport::HashWithIndifferentAccess(#70330196377080)):

app/controllers/users_controller.rb:7:in `signup'

And I'm not sure which error is preferable

jrhorn424 commented 7 years ago

I prefer your first data format.

But I don't think you should do it this way. The UsersController should only create a new user. The client can enforce the fact that you need the athlete profile completed.

In your client, after the user creation step completes, you can use a standard action by posting to athlete#create. Be sure to put this second API call into a "then", or use Promise.all instead.

As a further step, for things associated with Athlete, you can validate the requisite foreign key (in the API) and when it is missing, show a helpful message to the user.

cc @bernardlee

bernardlee commented 7 years ago

I prefer the first method as well. The reason that you're getting {"password":["can't be blank"]} is because the hashes being passed into your #create methods don't have the correct shape. Look at the structure of the hash being passed in and compare it to the structure of a hash being passed into a working invocation of #create.

@jrhorn424 What's a better way to do this? I was suggesting this since it was also how I remember learning how to do such a thing when I took the course. I had also considered your method and others, but creating and attaching a profile-like entity to a user account upon creation on the back end (if it's valid) seemed to make the most sense to me.

Edit: Removed repetition.

laurpaik-zz commented 7 years ago

I solved this by separating my params:

  def signup
    user = User.create(user_creds)
    if user.valid?
      @athlete = user.create_athlete(athlete_params)
      render json: user, status: :created
    else
      render json: user.errors, status: :bad_request
    end
  end
  def user_creds
    params.require(:credentials)
          .permit(:email, :password, :password_confirmation)
  end

  def athlete_params
    params.require(:athlete)
          .permit(:given_name, :surname, :date_of_birth)
  end

I'm also curious about what you're saying @jrhorn424 ... are you saying it's less secure to do it this way? I had tried using some .then's earlier before my one on one, but I guess I had trouble building something asynchronous (async because the token has to get made first, I'm guessing), or I approached it weirdly.

jrhorn424 commented 7 years ago

No, there's nothing about what you're doing that's insecure. I don't like building associated records inside the User controller. It feels like a (minor) violation of separation of concerns.

@laurpaik None of the advice below is necessary, but it may provide interesting reading if you care to google some of the weird words. :smile:

@bernardlee I'd still validate the existence on the backed, but you don't have to marry the entities to do that.

If you desire tighter coupling because it achieves greater semantic meaning or cohesion, that's fine. But in such a case I'd create a special model and controller to take care of it (hexagonal architecture, or DCI). These sorts of objects are called Presenters, if I remember correctly.

EDIT: Presenters are for complex views. My mistake. There's a special name for these kinds of interaction objects, but you can read more about DCI here https://en.wikipedia.org/wiki/Data,_context_and_interaction

raq929 commented 7 years ago

Please close this issue if it's resolved!

laurpaik-zz commented 7 years ago

Thank you!