Home > Enterprise >  Rails: NoMethodError (undefined method `update' for nil:NilClass)
Rails: NoMethodError (undefined method `update' for nil:NilClass)

Time:02-16

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 adding before_action :authenticate in your ApiController (or whatever the base class is) and then using skip_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.

  • Related