Ruby on Rails controller design

Question

When I look at examples of Rails controllers, I usually see something like this:

class WidgetController < ActionController::Base

  def new
    @widget = Widget.new
  end

  def create
    @widget = Widget.new(params[:id])
    if @widget.save
      redirect_to @widget
    else
      render 'new'
    end
  end
end

This works, but there's a couple problems:

Routes

If I add widgets to my routes.rb file:

Example::Application.routes.draw do
  resources :widgets
end

GET /widgets/new will route to new and POST /widgets will route to create.

If the user enters incorrect information on the new widget page and submits it, their browser will display a URL with /widgets, but the new template will be rendered. If the user bookmarks the page and returns later or refreshes the page, the index action will be called instead of the new action, which isn't what the user expects. If there's no index action or if the user doesn't have permission to view it, the response will be a 404.

Duplication of code

As a contrived example, let's say I had some tricky logic in my new method:

def new
  @widget = Widget.new
  do_something_tricky()
end

Using the current approach, I'd duplicate that logic in new and create. I could call new from create, but then I'd have to modify new to check if @widget is defined:

def new
  @widget ||= Widget.new
  do_something_tricky()
end

Plus, this feels wrong because it reduces the orthogonality of the controller actions.

What to do?

So what's the Rails way of resolving this problem? Should I redirect to new instead of rendering the new template? Should I call new inside of create? Should I just live with it? Is there a better way?

Answer 1

I don't think this is a problem in "the rails way" and there is no builtin functionality to allow this without getting your hands dirty. What does a user expects when bookmarking a form they just submitted and had errors? Users don't know better, and they shouldn't bookmark a failed form.

I think redirecting to new_widget_path is the cleanest solution. Yet, you should keep the errors and display them on the form. For this I recommend you keep the params in session (which I expect to be smaller than a serialized Widget object).

def new
  @widget = widget_from_session || Widget.new 
end

def widget_from_session
  Widget.new(session.delete(:widget_params)) if session[:widget_params].present?
end
private :widget_from_session

# Before the redirect
session[:widget_params] =  params

The code is self explanatory, Widget.new will only be called when widget_from_session returns nil, this is when session[:widget_params] is present. Calling delete on a hash will return de deleted value and delete it from the original hash.

UPDATE Option 2 What about submitting the form using ajax? Your controller could benefit from:

  respond_to :html, :json

  ...

  def create
    @widget = Widget.new params[:widget]
    @widget
    respond_with @widget, location: nil
  end

Based on the response code (which is set by Rails: 201 Created or 422 Unprocessable Entity), you could show the errors (available in the body of the response when validations fail) or redirect the user to @widget

This is how StackOverflow does it: https://stackoverflow.com/questions/ask. They submit the form asynchronously.

Answer 2

I have this problem before, so I use edit action instead.

Here is my code.

Routes:

resources :wines do
  collection do
    get :create_wine, as: :create_wine
  end
end

Controller:

def create_wine
  @wine = Wine.find_uncomplete_or_create_without_validation(current_user)
  redirect_to edit_wine_path(@wine)
end

def edit
  @wine = Wine.find(params[:id])
end

def update
  @wine = Wine.find(params[:id])
  if @wine.update_attributes(params[:wine])
    redirect_to @wine, notice: "#{@wine.name} updated"
  else
    render :edit
  end
end

Model:

def self.find_uncomplete_or_create_without_validation(user)
  wine = user.wines.uncomplete.first || self.create_without_validation(user)
end

def self.create_without_validation(user)
  wine = user.wines.build
  wine.save(validate: false)
  wine
end

View:

= simple_form_for @wine, html: { class: 'form-horizontal' } do |f|
  = f.input :complete, as: :hidden, input_html: { value: 'true' }

What I did is create a new action 'create_wine' with get action.

1. If user request 'create_wine', it will create a new wine without validation and redirect to edit action with a update form for attributes and a hidden field for compele .
2. If user has create before but gave up saving the wine it will return the last uncompleted wine.

Which means whether use save it or not, the url will be the same to /wines/:id.

Not really good for RESTful design, but solve my problem. If there is any better solution please let me know.

Answer 3

You don't need to write same function in two action , use before_filter instead.

If you want to have "widget_new_url" after incorrect submission then in your form add url of new widget path something like :url => widget_new_path .

Rails takes the url from Form .

Answer 4

you put do_something_tricky() in its own method and call it inside the create action (but only when you're rendering the new template, ie when validation fails).

As for the bookmark issue, I don't know a good way to prevent that but to modify the routes and set the create action to the new action but using POST

get '/users/new' => 'users#new'
post '/users/new' => 'users#create'

UPDATE: using resources

resources :platos, except: :create do
  post '/new' => 'plates#create', on: :collection, as: :create
end

then you can use create_platos_path in your forms

Answer 5

In general, I think the Rails way of solving the problem would be to put the tricky method onto the model or as a helper method, so the controller stays "thin" and you don't have to make sure to add custom behavior to both #new and #create.

EDIT: For further reading, I'd recommend the "Rails AntiPatterns" book, as they go through a lot of these common design issues and give potential solutions.