I am following this tutorial in YT:
https://www.youtube.com/watch?v=oyjzi837wME&list=PLWqjhA7WxVW6L7AWzQElmYfXV3NUv_Lbs&index=1
def update
airline = Airline.find_by(slug: params[:slug])
if airline.update(airline_params)
render json: AirlineSerializer.new(airline, options).serialized_json
else
render json: {error: "airline not "}, status: 422
end
end
But I am getting this error:
NoMethodError (undefined method `update' for nil:NilClass)
Any help would be greatly appreciated.
CodePudding user response:
In controllers you almost always want to use find
or find_by!
so that an ActiveRecord::RecordNotFound
exception is raised and you bail early instead of getting a nil error:
def update
airline = Airline.find_by!(slug: params[:slug])
if airline.update(airline_params)
render json: AirlineSerializer.new(airline, options).serialized_json
else
render json: { errors: airline.errors.full_messages }, status: 422
end
end
After all if the record can't even be found there is no reason to continue processing - which is something the tutorial author didn't consider or test for.
This exception is rescued by Rails by default which sends a 404 response but you can override it on a per controller basis by using rescue_from:
class AirlinesController < ApplicationController
rescue_from ActiveRecord::RecordNotFound, with: :not_found
# ...
private
def not_found
render json: { error: "Oh noes" }, status: :not_found
end
end
On a side note params[:slug]
is a bit of an antipattern. Your routes should just stick with the conventional /airlines/:id
. :id
in this case just means a unique identifier for the resource and not the id column. This avoids the need to refactor if you want to look records up by id or slug.
There are also some other problems with the tutorial code:
- Authentication is opt-in as evidenced by
before_action :authenticate, only: %i[create update destroy]
. Use a opt-out secure by default approach instead so that you don't inadvertantly leave security holes. You do this by addingbefore_action :authenticate
in yourApiController
(or whatever the base class is) and then usingskip_before_action :authenticate, ...
to opt out on endpoints that should not require authentication. - There is no validation of slugs and no guarentee of uniqueness for either the slug or name in the form of unique indexes. Just validating the name doesn't actually guarentee the uniqueness of the slug as the name can be changed but the slug is generated when the record is first created.
parameterize
is a quite naive solution to the problem of slugging. For real world use cases you need a library like stringex. Or use FriendlyID if you want to avoid reinventing the wheel.
CodePudding user response:
I have spotted my mistake in following the tutorial. The code in the route should be as below:
namespace :api do
namespace :v1 do
resources :airlines, param: :slug
resources :reviews, only: [:create, :destroy]
end
end
Instead I have put resources :airlines, params: :slug
with an s in the param.
So I am not very sure why there is no warning about this error when I tried to run it.