Rails: half-restful routes

Question

I've got these three bottom routes below which are very error-prone because of other routes declared normally:

# normal routes
resources :documents, :except => [:show, :edit, :update]
resources :photos, :except => [:show, :index]
...
# error-prone routes
get ":client_code" => "share#index", :as => :shares, :format => false
get ":client_code/:id" => "share#show", :as => :share, :format => false
get ":client_code/:document_id/more/:component_id" => "share#more", :as => :more, :format => false

I've got a few methods in the ShareController to deal with the requests like so:

def show
  get_user_by_parameter
  if get_document_by_user_or_issue and @document.is_showable? and @parameter_user == @document.user
    ...
end

private

def get_user_by_parameter
  @parameter_user = User.where(:client_code => params[:client_code]).first
end


def get_document_by_user_or_issue
  if params[:id].match(/\D/)
    @document = Document.where(:user_id => @user.id, :issue => params[:id]).first
  else
    @document = Document.find(params[:id])
  end
end

I need the routes to be that minimal, but not only is this ugly and un-RESTful but it's very error prone.

The :client_code will always be the owner of the @document being viewed. It's kinda like a safety check/ownership kinda function. But, because of all the reasons listed above: is there a better way to write this? There's gotta be a better way than that.

Thanks.

Answer 1

Controller Based Check:

before_filter :find_document

def find_document
  Document.find(params[:id])
end

def is_owner?(document)
 redirect_to root_path if current_user.id != document.owner_id
end

Isn't a check like this much easier? I'm not sure why you have a share controller, so I don't want to be presumptuous here.

Which will allow you to do:

resources :shares, only: [:index, :show]

Also:

 User.where(:client_code => params[:client_code]).first

Can be refactored to:

User.find_by(client_code: params[:client_code])

Assuming you are on latest rails version, else:

User.find_by_client_code(params[:client_code])

Let me know what the shares are for, I'm not sure I provided the full solution for you.

Cheers.

EDIT

if you are using shares to provide a different view, i suggest doing this:

Within the controller,

def index
  if params[:shares]
    render 'shares'
  end
end

Unless you really wish to have a different route for it. This allows you to not have a shares controller for essentially what is the document model.