Undefined Method 'reviews_path' for #<#:0x51970e0>

Question

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 &#9734;<%= f.radio_button :stars, '1' %>
        2 &#9734;<%= f.radio_button :stars, '2' %>
        3 &#9734;<%= f.radio_button :stars, '3' %>
        4 &#9734;<%= f.radio_button :stars, '4' %>
        5 &#9734;<%= 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.

Answer 1

This is just an extended version of @japed answer.

1. You have no route to the create or update action

Both 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.

2. The controller:

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.

3. The form:

<% 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.

4. Other links

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)

Answer 2

As you're doing nested routes, you need to find by :picture_id as you've just found

class ReviewsController < ApplicationController
  before_action { @picture = Picture.find(params[:picture_id] }
end

As your error says the issue is because reviews_path doesn't exist because you've nested it

So this

<% form_for @review do |f| %>

Wants to change to

<% form_for [@picture, @review] do |f| %>

So that it goes to the picture_reviews_path

Also this

<p><%= link_to 'New Review', new_reviews_path(@review), :class => "btn btn-info" %></p>

Wants to become

<p><%= link_to 'New Review', new_picture_reviews_path(@picture, @review), :class => "btn btn-info" %></p>

Answer 3

Can you use Shallow Nesting Routes? That is, you'll have a nested resource where needed, but when unambiguous you get a shorter path, with just one parameter for the review. You can still find your way back to the picture, using the picture_id in the review.

resources :pictures, shallow: true do
  put :favorite, on: :member
  resources :reviews, shallow: true
end

resources :users
resources :sessions, only: [:new, :create, :destroy]
resources :contacts, only: [:new, :create]

Then, improve the models to help the associations to bind well, with inverse_of:

class Review < ActiveRecord::Base
  belongs_to :picture, inverse_of: :reviews
end

class Picture < ActiveRecord::Base
  has_many :reviews, inverse_of: :picture
end

This should mean there's only one copy of a picture in memory. And then in the ReviewsController:

class ReviewsController < ApplicationController
  before_action :set_review, only: [:show, :edit, :update, :destroy]

  def index
    @picture = Picture.find(params[:id])
    # reference @picture.reviews to get all reviews in the view
  end      

  def show
    @picture = Picture.find(params[:id])
    # use @picture.reviews to get all reviews in the view
  end

  def new
    # where will you get the picture this belongs to? 
    # Need to collect the picture_id param. and build the associated review
    @picture = Picture.find(param[:picture_id])
    @review = @picture.reviews.build()
  end

  def edit
     @picture = Picture.find(params[:picture_id])
     # use @picture.reviews in the view controller to get the associated reviews
  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

I think there's one other significant issue. You keep using a piece of code like this:

@review = Picture.find(id)

But that returns zero or more elements. It will help you understand the code better if you reflect that this is, normally, an array:

@reviews = Picture.find(id)

But even better, don't do that. You have the associations. Use them in the view.

@picture.reviews

This will return an array. If zero length, there are no reviews. If non-zero, that's how many review elements there are.

Then you won't make the mistake of picking up an array variable called @review, which appears to be singular (meaning that link_to @review appears to make sense, but will fail), and instead use an array:

<%- @picture.reviews.each do |review| %>
 <% link_to review ...%>

Hope that helps!