Rails Way to handle action that's available on multiple routes

Question

I have the following routes:

resources :users do
  # List reviews made by user
  resources :reviews, :only => [ :index ]
end

resources :products do
  # List reviews by product, and provide :product_id for creation
  resources :reviews, :only => [ :index, :new, :create ]
end

# Other actions don't depend on other resources
resources :reviews, :except => [ :index, :new, :create ]

Everything looks right, except ReviewsController#index:

def index
  if params[:user_id]
    @reviews = Review.find_all_by_user_id params[:user_id]
  else
    @reviews = Review.find_all_by_product_id params[:product_id]
  end
  respond_with @reviews
end

I was wondering if there's a standard solution to the problem above, or if there is a better way to do it.

Answer 1

What you have there is fine, but if you want you can use two different actions as well. This approach should allow you to more easily change the view later on, and is a bit safer.

match '/products/:product_id/reviews' => 'reviews#product_index'
match '/users/:user_id/reviews' => 'reviews#user_index'

It will also keep your controller code a little cleaner and less susceptible to odd queries like /products/10/reviews?user_id=100 which would result in the user's reviews being shown instead of the product's reviews.

def product_index
  @reviews = Review.find_all_by_product_id params[:product_id]
  respond_with @reviews
end

def user_index
  @reviews = Review.find_all_by_user_id params[:user_id]
  respond_with @reviews
end

The other alternative is to use different controllers as well:

match '/products/:product_id/reviews' => 'product_reviews#index'
match '/users/:user_id/reviews' => 'user_reviews#index'

Answer 2

Some plugins have ways to load resources for you, such as declarative_authorization or cancan, I'm sure there are others.

Other solutions I have seen is to make a private method in the controller to load the object, and in that method is essentially the same logic you have here; it just moves it out of the index action itself. The metod can also then be called as a before filter.

One other way to do your logic is to start with the parent object (nice if you need the parent object too:

before_filter :load_collection, :only => :index

private
def load_collection
  if params[:user_id]
    @user = @parent = User.find(params[:user_id])
  else
    @product = @parent = Product.find(params[:product_id])
  end
  @reviews = @parent.reviews
end