References and External Links
Ruby on Rails - Settting up Reviews functionality
NoMethodError in Discussions#new
http://ruby.about.com/od/rubyonrails/ss/blogpart4_4.htm
Background
I'm implementing a feature in my application that allow users to rate and review pictures.
I am using a Posts/Comments relationship model for a Pictures/Reviews relationship.
Models
class Review < ActiveRecord::Base
belongs_to :picture
end
class Picture < ActiveRecord::Base
has_many :reviews
end
Above, I established a one-to-many relationship between pictures and reviews.
Reviews Migration
class CreateReviews < ActiveRecord::Migration
def change
create_table :reviews do |t|
t.string :username
t.text :body
t.references :picture, index: true
t.timestamps
end
end
end
Matched Routes
match '/pictures/:id/reviews', to: 'reviews#show', via: 'get', :as => 'picture_reviews'
match '/pictures/:id/reviews/edit', to: 'reviews#edit', via: 'get'
match '/pictures/:id/reviews/new', to: 'reviews#new', via: 'get', :as => 'new_reviews'
I will name the route for reviews#edit after I fix this issue with reviews#new.
Error Message
NoMethodError in Reviews#new
Undefined method 'reviews_path' for #<#<Class:0x45c1b00>:0x39ae810>
Extracted source (Around line #8):
5 <div class = 'edit-form'>
6 <div class = 'center'>
7
8 <% form_for @review do |f| %>
9
10 <p>
11 <%= f.label :username %><br />
I checked to see if any files contained 'review-path', but all routes were properly named.
Routes
favorite_picture_path PUT /pictures/:id/favorite(.:format) pictures#favorite
pictures_path GET /pictures(.:format) pictures#index
POST /pictures(.:format) pictures#create
new_picture_path GET /pictures/new(.:format) pictures#new
edit_picture_path GET /pictures/:id/edit(.:format) pictures#edit
picture_path GET /pictures/:id(.:format) pictures#show
PATCH /pictures/:id(.:format) pictures#update
PUT /pictures/:id(.:format) pictures#update
DELETE /pictures/:id(.:format) pictures#destroy
users_path GET /users(.:format) users#index
POST /users(.:format) users#create
new_user_path GET /users/new(.:format) users#new
edit_user_path GET /users/:id/edit(.:format) users#edit
user_path GET /users/:id(.:format) users#show
PATCH /users/:id(.:format) users#update
PUT /users/:id(.:format) users#update
DELETE /users/:id(.:format) users#destroy
sessions_path POST /sessions(.:format) sessions#create
new_session_path GET /sessions/new(.:format) sessions#new
session_path DELETE /sessions/:id(.:format) sessions#destroy
contacts_path POST /contacts(.:format) contacts#create
new_contact_path GET /contacts/new(.:format) contacts#new
root_path GET / pictures#welcome
users_new_path GET /users/new(.:format) users#new
about_path GET /about(.:format) pictures#about
GET /contacts(.:format) contacts#new
GET /users/:id/favorites(.:format) users#favorites
signup_path GET /signup(.:format) users#new
signin_path GET /signin(.:format) sessions#new
signout_path DELETE /signout(.:format) sessions#destroy
picture_reviews_path GET /pictures/:id/reviews(.:format) reviews#index
GET /pictures/:id/reviews/edit(.:format) reviews#edit
new_reviews_path GET /pictures/:id/reviews/new(.:format) reviews#new
updated_path GET /updated(.:format) pictures#new_updates
GET /top-rated(.:format) pictures#high_ratings
ReviewsController (Part 1)
class ReviewsController < ApplicationController
before_action :set_review, only: [:show, :edit, :update, :destroy]
def index
@picture = Picture.find(params[:id])
@review = Review.all
end
def show
@picture = Picture.find(params[:id])
@review = Review.find(params[:id])
end
def new
@review = Review.new
end
def edit
@picture = Picture.find(params[:picture_id])
@review = Review.find(params[:id])
end
def create
@picture = Picture.find(params[:picture_id])
@review = @picture.reviews.build(params[:review])
if @review.save
flash[:notice] = 'Review was successfully created.'
redirect_to @picture
else
flash[:notice] = "Error creating review: #{@review.errors}"
redirect_to @picture
end
end
Reviews Controller(Part 2)
def update
@picture = Picture.find(params[:picture_id])
@review = Review.find(params[:id])
if @review.update_attributes(params[:review])
flash[:notice] = "Review updated"
redirect_to @picture
else
flash[:error] = "There was an error updating your review"
redirect_to @picture
end
end
def destroy
@picture = Picture.find(params[:picture_id])
@review = Review.find(params[:id])
@review.destroy
redirect_to(@review.post)
end
private
def set_review
@review = Review.find(params[:id])
end
def review_params
params.require(:review).permit(:username, :body, :picture_id)
end
end
Reviews#Index Page
<h3>Reviews for <%= "#{@picture.title}" %></h3>
<table>
<thead>
</thead>
<tbody>
</tbody>
</table>
<div class = 'center'>
<p><%= link_to 'New Review', new_reviews_path(@review), :class => "btn btn-info" %></p>
<p><%= link_to 'Back', picture_path, :class => "btn btn-info" %></p>
</div>
Link to the Reviews#new page
<p><%= link_to 'New Review', new_reviews_path(@review), :class => "btn btn-info" %></p>
Reviews#New Page
<% @title = "New Review" %>
<h3>New Review</h3>
<div class = 'edit-form'>
<div class = 'center'>
<% form_for @review do |f| %>
<p>
<%= f.label :username %><br />
<%= f.text_field :username %>
</p>
<p>
<%= f.label :body %><br />
<%= f.text_area :body %>
</p>
<p>
<%= f.submit "Submit Review" %>
</p>
<% end %>
</div>
</div>
<div class = 'center'>
<%= link_to 'Back', picture_reviews_path(@picture) %>
</div>
Pictures#Show Page
<% @title = "#{@picture.title}" %>
<h4 class = 'indent'>Picture Statistics</h4>
<ul id = 'view'>
<li><strong>Title:</strong> <%= @picture.title %></li>
<li><strong>Category:</strong> <%= @picture.category %></li>
<li><strong>Rating:</strong> <%= pluralize(@picture.rating, 'Star') %></li>
<li><strong>Favorited:</strong> By <%= pluralize(@picture.users.count, 'User') %></li></br>
</ul>
<% if @picture.rating > 4 %>
<button class = 'top-picture'>Top Rated</button>
<% end %>
<%= form_for @picture do |f| %>
<div class = 'indent'>
<p>
<%= f.label :stars, 'Rating' %>
<div class= "rating">
1 ☆<%= f.radio_button :stars, '1' %>
2 ☆<%= f.radio_button :stars, '2' %>
3 ☆<%= f.radio_button :stars, '3' %>
4 ☆<%= f.radio_button :stars, '4' %>
5 ☆<%= f.radio_button :stars, '5' %>
</div>
</p>
<p><input class="btn btn-info" type="submit" value="Rate"></p>
<p><%= link_to 'Reviews', picture_reviews_path(@picture), :class => "btn btn-info" %></p>
<% end %>
<p><%= link_to 'Index', pictures_path, :class => "btn btn-info" %></p>
</div>
I've tried using nested resources like so
resources :pictures do
put :favorite, on: :member
resources :reviews
end
resources :users
resources :sessions, only: [:new, :create, :destroy]
resources :contacts, only: [:new, :create]
That didn't work because It routed my pictures using :picture_id instead of the standard :id field. Since it routed to :picture_id it couldn't find any pictures.
picture_reviews_path GET /pictures/:picture_id/reviews(.:format) reviews#index
GET /pictures/:picture_id/reviews/edit/:id(.:format) reviews#edit
new_reviews_path GET /pictures/:picture_id/reviews/new(.:format) reviews#new
Picture Columns
Picture.column_names
=> ['id', 'title', 'category', 'stars', 'created_at', 'updated_at',
'ratings_count', 'ratings_total']
The problem with nesting routes, is that it calls a path using a column_name not found in the table. That is why I decided to go back to matching routes.
I believe the problem lies in my ReviewsController for which there may be duplicated code.
before_action :set_review, only: [:show, :edit, :update, :destroy]
@review = Review.find(params[:id])
def set_review
@review = Review.find(params[:id])
end
I think I could remove the @review = Review.find line from every method, but my main concern is that the set_review method was defined as a private method so that might not be possible.
Help is greatly appreciated and thanks in advanced.
Update
I think the problem lies in my new action in my ReviewsController.
This is just an extended version of @japed answer.
create or update actionBoth actions works on POST request, hence url_helpers alone won't tell rails what to do with POST request when it gets it. What you need is to change your routes back to nested resources (it was good the way it was, your issue was caused by another bit of code). So, you need:
resources :pictures do
...
resources :reviews
end
Also remove all other routes for this controller as they may affect your final routes. Remeber to restart your server after changing your routes.
Firstly, note that there are a lot of repetitions there - you are setting @picture in all the actions. Currently your problem is that it is using params[:id] in some actions and params[:picture_id] in others. It should always be picture_id, id should be reserved to be review's id, as you are inside reviews_controller.
The best way to do this is to create another before_filter which will set up the @picture variable:
class ReviewsContorller < ApplicationController
before_filter :set_picture
# This is perfectly fine, but needs to be executed after :set_picture
before_filter :set_review, only: [:show, :edit, :update, :destroy]
...
private
...
def set_picture
@picture = Picture.find(params[:picture_id])
end
def set_review
@review = picture.reviews.find(params[:id])
end
end
Note that the @review is pulled from @picture association - this is important security check, if you used Review.find instead, all the users are automatically able to view, edit and create new reviews for all the photos, without knowing which photo they are really commenting for. It should not be a great issue in your case, but it is good to get this into the habit.
<% form_for @review do |f| %>
This would seems all right, however imagine you are your application - how would you know what is the correct post url for this form? Rails is quite intelligent framework and it is trying to guess it by the resource supplied. In this case, you pass an instance of Review class, hence it will try to send the from to review_path(@review.id). The problem is, that this path does not exists in your routes, so you will get undefined_method 'review_path' here.
Also note, that the proper route you want is /picture/:picture_id/reviews for new reviews or /picture/:picture_id/review/:idfor existing reviews. Hence rails will need the parent picture object to be passed as well to figure out the rightpicture_id`. You can do this by passing an array of resources, with the one which the form is really for being the last so:
<% form_for [@picture, @review] do |f| %>
This will tell rails to look for picture_reviews_path(@picture.id) for new review or picture_review_path(@picture.id, @review.id) for existing reviews. If you have nested resources in your routes, both of those should exists.
Your current routes defines a named path new_reviews which will not longer exist after you use nested resources - it will be renamed to new_picture_review, so you need to change all the occurrences of new_reviews_path to new_picture_review(@picture)