views:

134

answers:

4

This happened to me several times and I'm yet to find an acceptable solution.

I have a form in the home page of a site, that forms points to another controller which actually does the job of processing the data. When the form is submitted successfully, the other controller sends you back to the homepage with a nice flash[:notice] message, and that's the end of it.

When there's a validation issue it becomes a problem. What I'd like to do is to show the homepage with the form with the validation errors. The naive solution normally mention is that you can render another template, but showing the homepage is much more than rendering a template, it has a lot of functionality. The only way to render that template is to copy and paste the functionality in this other controller action, or maybe taking all functionality away from a controller, which is not very nice either

Is there a better solution?

Update: I understand people saying that the controller actions should be smaller and call another method, but in practice, I don't see how to achieve it. I'm going to post a real example out of a web site I did.

There are two models and controllers: posts and comments. A post has many reasons. An post is shown this way:

def index
  set_posts  # sets @posts
end

def show
  @post = Post.find_by_slug(params[:id])
  @comment = Comment.new
  if not @post
    flash[:error] = "'#{params[:id]}' does't exist"
    set_posts
    render :action => :index, :status => :not_found
  end
end

private
def set_posts
   @posts = Posts.get_all_public_posts
end

The comments controller only has a create action:

def create
  @comment = Comment.new(params[:comment])
  @comment.post = Post.find_by_slug params['post_id']

  if not @comment.post
    # Now what?
    # We should here call PostsController.set_posts and render views/posts/index
  end

  if @reason.save
    flash[:notice] = 'Thank you for your message.'
    redirect_to(@reason.item)
  else
    # Now what?
    # We should here call PostsController.show without overriding the @comment
  end
end

end

The "Now what?" parts being the ones I don't have a good solution for.

A: 

How is this diffrent from a normal vanilla rails form?

if(valid)
 flash = ...
 redirect_to :home
else
 rerender form with error messages & submitted values
end
Keynan
The difference is that there's a lot of controller logic required to render the form that's already contained in home page action.
EmFi
rerender form is missing about 25 lines of code that is on the home controller action.
J. Pablo Fernández
+1  A: 

How much does the rest of the home page change between when the form is rendered and the request is redirected to the default action?

If the answer is not much at all, then you should consider using remote_form_for and update only the notification area on success or the form with validation errors on failure.

If that's not to your liking, you could move all the repeated logic in the home page action to a method defined in the ApplicationController class, and invoke it as part of a before_filter on the home page action and the action that processes your form. N.B: doing things this way will require you to set instance variables, local variables set in a filter will not persist until an action.

EmFi
That could be one possible solution. Thanks.
J. Pablo Fernández
@J. Pablo Fernandez: I've edited my solution to contain another way of doing it.
EmFi
+1  A: 

I've encountered this issue before. I came from CakePHP, where validation errors are stored in the session and persisted between requests. Rails does not behave this way by default, which leaves you with a decision to make on how to handle errors.

As you have apparently also read, putting the errors into the session and doing a redirect is generally not advised in the Rails world. And as you say, the Rails way seems to be to simply render another action without doing a redirect. When I first attempted that, I noticed that I was forced to duplicate a lot of code in order to set up the second action to render the first action's view.

Just as with any type of code duplication, the solution is to move the duplicated code into a separate method and then call that method from the two actions. A clean way to do this is to use a before_filter to run the code for both actions.

Jimmy Cuadra
How do you move it to a separate method when you are interacting across controllers, like in my now updated example? Using before_filter would mean doing extra uneccesarry queries just in case, wouldn't it?
J. Pablo Fernández
Not sure I understand your code... What is this `@reason` variable coming out of nowhere in your `Comments#create` action? You're suddenly attempting to save something that doesn't even exist. Also, the `if not @post` blocks will not do what you're wanting. You need to rescue `ActiveRecord::RecordNotFound` on the line that fetches the post(s).
Jimmy Cuadra
A: 

I concur with the other posters here - if you have similar code it's prime for refactoring into a common method called by each of the controller actions.

Application Controller is the best place to put it if it's common across multiple controllers.

Alternatively you could make a common controller class that both of the controllers inherits from - and which only contains that helper method. I don't recommend this unless you are likely to share other methods as well.

Taryn East