ga-wdi-boston / capstone-project

Other
3 stars 28 forks source link

Create Crud breaking #843

Closed mak1323 closed 7 years ago

mak1323 commented 7 years ago

My rails api is nesting the create rout for a resource through another. I've gotten the read actions to work this way, now I need to get the create action. It is more or less hitting the correct api, but I am returning a broken response when creating.

The docs say this is my route for post: api_source/campaigns/:campaign_id/capaign_logs

So I ran back with this curl script:

#!/bin/bash

API="${API_ORIGIN:-http://localhost:4741}"
URL_PATH="/campaigns/${ID}/campaign_logs"
curl "${API}${URL_PATH}" \
  --include \
  --request POST \
  --header "Content-Type: application/json" \
  --header "Authorization: Token token=$TOKEN" \
  --data '{
    "campaign_log": {
      "title": "'"${TITLE}"'",
      "log": "'"${LOG}"'"
    }
  }'

echo

which parses against the backend with this input:

$ TOKEN="BAhJIiU5NjNmMmI2YWViZDc4MTZmZWMyNzE5Njg1YzA4ZjkwNAY6BkVG--fade694c39c5deda4d779b3c821512eee6832efe" ID=4 TITLE="Log" LOG="Test" sh scripts/log/create-log.sh 

as

Processing by CampaignLogsController#create as */*
  Parameters: {"campaign_log"=>{"title"=>"Log", "log"=>"Test"}, "campaign_id"=>"4"}
   (0.2ms)  BEGIN
   (0.2ms)  ROLLBACK
[active_model_serializers] Rendered ActiveModel::Serializer::Null with ActiveModel::Errors (0.06ms)
Completed 422 Unprocessable Entity in 3ms (Views: 0.6ms | ActiveRecord: 0.4ms)

The response I'm getting on console is:

HTTP/1.1 422 Unprocessable Entity
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: a9c2be51-7eec-4b1a-bffb-85232b914d7a
X-Runtime: 0.004722
Vary: Origin
Transfer-Encoding: chunked

{"campaign":["must exist"]}

I'm not sure what it means as {"campaign":["must exist"]} because I have triple checked that campaign 4 is in fact existing and ready to go. The response is the same for every input of an existing campaign.

MicFin commented 7 years ago

Can you share your schema?

It appears like when you are trying to create a campaign_log it is telling you that you must also give it a campaign to be associated with. When creating a campaign_log a campaign must exist.

mak1323 commented 7 years ago

Entire schema is

ActiveRecord::Schema.define(version: 20170829205657) do

  # These are extensions that must be enabled in order to support this database
  enable_extension "plpgsql"

  create_table "campaign_logs", force: :cascade do |t|
    t.integer  "campaign_id"
    t.string   "log"
    t.datetime "created_at",  null: false
    t.datetime "updated_at",  null: false
    t.string   "title"
    t.index ["campaign_id"], name: "index_campaign_logs_on_campaign_id", using: :btree
  end

  create_table "campaigns", force: :cascade do |t|
    t.integer  "user_id"
    t.string   "name"
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
    t.boolean  "completed"
    t.index ["user_id"], name: "index_campaigns_on_user_id", using: :btree
  end

  create_table "examples", force: :cascade do |t|
    t.text     "text",       null: false
    t.integer  "user_id",    null: false
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
    t.index ["user_id"], name: "index_examples_on_user_id", using: :btree
  end

  create_table "users", force: :cascade do |t|
    t.string   "email",           null: false
    t.string   "token",           null: false
    t.string   "password_digest", null: false
    t.datetime "created_at",      null: false
    t.datetime "updated_at",      null: false
    t.index ["email"], name: "index_users_on_email", unique: true, using: :btree
    t.index ["token"], name: "index_users_on_token", unique: true, using: :btree
  end

  add_foreign_key "campaign_logs", "campaigns"
  add_foreign_key "campaigns", "users"
  add_foreign_key "examples", "users"
end
MicFin commented 7 years ago

Can you share your controller?

mak1323 commented 7 years ago

Which one or both?

MicFin commented 7 years ago

CampaignLogsController

mak1323 commented 7 years ago

campaign_logs

# class CampaignLogsController < ProtectedController
class CampaignLogsController < ApplicationController
  before_action :set_campaign_log, only: [:show, :update, :destroy]

  # GET /campaign_logs
  def index
    @campaign_logs = CampaignLog.all

    render json: @campaign_logs
  end

  # GET /campaign_logs/1
  def show
    render json: @campaign_log
  end

  # POST /campaign_logs
  def create
    @campaign_log = CampaignLog.new(campaign_log_params)

    if @campaign_log.save
      render json: @campaign_log, status: :created, location: @campaign_log
    else
      render json: @campaign_log.errors, status: :unprocessable_entity
    end
  end

  # PATCH/PUT /campaign_logs/1
  def update
    if @campaign_log.update(campaign_log_params)
      render json: @campaign_log
    else
      render json: @campaign_log.errors, status: :unprocessable_entity
    end
  end

  # DELETE /campaign_logs/1
  def destroy
    @campaign_log.destroy
  end

  private
    # Use callbacks to share common setup or constraints between actions.
    def set_campaign_log
      @campaign_log = CampaignLog.find(params[:id])
    end

    # Only allow a trusted parameter "white list" through.
    def campaign_log_params
      params.require(:campaign_log).permit(:campaign_id, :log, :title)
    end
end
mak1323 commented 7 years ago

I originally had the create set to current_user.campaigns.find(params[:id]).campaign_logs.build(campaign_log_params) with a protected controller, but ran into more difficulties,

MicFin commented 7 years ago

Your create action needs to take into consideration the Campaign and the campaign_id that you pass with api_source/campaigns/:campaign_id/campaign_logs.

I would recommending setting up another before action that uses current_user for ownership.

  before_action :set_campaign, only: [:create]

  # POST /campaigns/:campaign_id/campaign_logs
  def create
    @campaign_log = @campaign.campaign_logs.build(campaign_log_params)

    if @campaign_log.save
      render json: @campaign_log, status: :created, location: @campaign_log
    else
      render json: @campaign_log.errors, status: :unprocessable_entity
    end
  end

private

def set_campaign
      @campaign = current_user.campaigns.find(params[:campaign_id])
end
mak1323 commented 7 years ago

I'm not sure how to do that with a nested route; I'm not sure if the relationship data of has_many :campaign_logs, through: :campaigns works the same way for setting current_user on the before action. My routes are

# frozen_string_literal: true
Rails.application.routes.draw do
  # resources :campaigns do
  #   resources :campaign_logs, only: [:index, :create]
  # end
  # resources :campaign_logs, only: [:show, :update, :delete]

  resources :campaigns, except: [:new, :edit] do
    resources :campaign_logs, only: [:index, :create, :new]
    end
  resources :campaign_logs, only: [:show, :update, :destroy]

  resources :examples, except: [:new, :edit]

  post '/sign-up' => 'users#signup'
  post '/sign-in' => 'users#signin'
  delete '/sign-out/:id' => 'users#signout'
  patch '/change-password/:id' => 'users#changepw'
  resources :users, only: [:index, :show]
end

I'm thinking at this point I should eliminate the nested route, and just make it a clean route and adding direct ownership from user and then set current user.

MicFin commented 7 years ago

Did you try what I posted above?

mak1323 commented 7 years ago

Playing with it now. I already had that mostly written before the response.

MicFin commented 7 years ago

Let me know how it goes and if you receive an error share it.

mak1323 commented 7 years ago

No luck, coming back a 500 error each time with the following error:

Started POST "/campaigns/4/campaign_logs" for 127.0.0.1 at 2017-08-30 10:53:35 -0400
Processing by CampaignLogsController#create as */*
  Parameters: {"campaign_log"=>{"title"=>"Log", "log"=>"Test"}, "campaign_id"=>"4"}
Completed 500 Internal Server Error in 1ms (ActiveRecord: 0.0ms)

NoMethodError (undefined method `campaign' for nil:NilClass):

app/controllers/campaign_logs_controller.rb:56:in `set_campaign'

I tried modifying that with several different params on set_campaign just to be sure it wasnt syntax.

mak1323 commented 7 years ago

changed over campaign to campaigns and back, modified paramas to :id and :campaign_id and back with both conditions. 500 error.

MicFin commented 7 years ago

You are inheriting from ApplicationController so current_user is not being defined.
You should inherit from OpenReadController or ProtectedController.
That is why it is saying in the set_campaign method that it can not call campaign method on nil:NilClass

NoMethodError (undefined method `campaign' for nil:NilClass):

app/controllers/campaign_logs_controller.rb:56:in `set_campaign'
mak1323 commented 7 years ago

Cool, your params worked. The final set campaign was

def set_campaign
      @campaign = current_user.campaigns.find(params[:campaign_id])
    end

With the nesting of the route, I had to call the current campaign by campaign_id instead of just :id.

I wasn't calling it right in the first place. Never would have found that, thank you.