CODEWITHSUNDEEP
ruby-on-railsruby-on-rails-3
t56k
t56k

Reputation: 6981

Returning a 404 if a user doesn't exist

Why would this not generate a 404 response?

def index
  if find_user
    @documents = @client.documents
    respond_to do |format|
      format.html
      format.atom { render layout: false }
    end
  else
    flash[:error] = "#{params[:client_code]} is not a client."
    render 'error', status: '404'
  end
end

def find_user
  @client = User.find_by_client_code(params[:client_code]) if valid_user?
end

def valid_user?
  User.all.each.map(&:client_code).include?(params[:client_code])
end

Like, if the code is incorrect it should return a 404, right? And not an exception? Can't quite get it to work.

EDIT: sorry, here's the error:

An ActionView::MissingTemplate occurred in share#index:
* Parameters : {"controller"=>"share", "action"=>"index", "client_code"=>"ampNDHEDD", "format"=>"atom"}

Upvotes: 0

Views: 281

Answers (3)

DGM
DGM

Reputation: 26979

First, your valid_user? method is a really bad idea - it loads the entire user database just to see if the code is present... which is the same result as what User.find_by_client_code does, but without loading every record! I'd just nuc the method and the if clause. If there is no matching record, it should return nil, which should take the else path and render the error.

As for why it's not rendering the error... I'm not sure if the atom format has anything to do with it, but when code doesn't branch the way I expect, I always put a Rails.logger.debug ... before the branch I have an issue with, and/or put a bad method in the branch it's supposed to take. That helps narrow it down. :D

Upvotes: 1

Matenia Rossides
Matenia Rossides

Reputation: 1414

If you don't use the valid_user? or the find_user methods elsewhere, they can be removed and you could do the following

def index
  @client = User.find_by_client_code(params[:client_code]) # returns nil or a record
  if @client
    @documents = @client.documents
    respond_to do |format|
      format.html
      format.atom { render layout: false }
    end
  else
    flash[:error] = "#{params[:client_code]} is not a client."
    render status: 404
  end
end

However, your previous comment states you're getting a template error which indicates that you may not have an index.atom template available to render.

Upvotes: 2

Paritosh Piplewar
Paritosh Piplewar

Reputation: 8122

do this 

def index
  if find_user
    @documents = @client.documents
    respond_to do |format|
      format.html
      format.atom { render layout: false }
    end
  else
    flash[:error] = "#{params[:client_code]} is not a client."
    raise ActionController::RoutingError.new('Not Found')
  end
end

def find_user
  @client = User.find_by_client_code(params[:client_code]) if valid_user?
end

def valid_user?
  User.where(client_code: params[:client_code]).present?
  // User.all.each.map(&:client_code).include?(params[:client_code]) // this is terrible (Read my comment )
end

Upvotes: 1

Related Questions