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.
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'
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
def index
key = [:user_id, :product_id].find{|k| params[k]}
@reviews = Review.where(key => params[key]).first
respond_with @reviews
end
精彩评论