Rails validation over redirect

Question

I'm trying out the beast forum written in rails and will use this as an example of a problem I keep facing.

The forum has a topics/show action and view with a form at the bottom to create a new post within the topic.

Submitting the form goes to posts/create and if the validation passes redirects back to topics/show and works fine, however if the validation fails (leaving out the body field) you're redirected to the same topics/show and back to the form, with no validation errors... normally if validation fails you're left on whatever/create with render :action => new.

Are the validations being lost in the redirect, and what's the best method of getting it working?

See code below:

PostsController.rb

  def create
    @post = current_user.reply @topic, params[:post][:body]

    respond_to do |format|
      if @post.new_record?
        format.html { redirect_to forum_topic_path(@forum, @topic) }
        format.xml  { render :xml  => @post.errors, :status => :unprocessable_entity }
      else
        flash[:notice] = 'Post was successfully created.'
        format.html { redirect_to(forum_topic_post_path(@forum, @topic, @post, :anchor => dom_id(@post))) }
        format.xml  { render :xml  => @post, :status => :created, :location => forum_topic_post_url(@forum, @topic, @post) }
      end
    end
  end

TopicsController.rb

  def show
    respond_to do |format|
      format.html do
        if logged_in?
          current_user.seen!
          (session[:topics] ||= {})[@topic.id] = Time.now.utc
        end
        @topic.hit! unless logged_in? && @topic.user_id == current_user.id
        @posts = @topic.posts.paginate :page => current_page
        @post  = Post.new
      end
      format.xml  { render :xml  => @topic }
    end
  end

topics/show view

  <% form_for :post, :url => forum_topic_posts_path(@forum, @topic, :page => @topic.last_page) do |f| %>

  <%= f.error_messages %>

  <table width="100%" border="0" cellpadding="0" cellspacing="0">
    <tr>
      <td rowspan="2" width="70%">
        <%= f.text_area :body, :rows => 8 %>
      </td>
      <td valign="top">
        <%= render :partial => "posts/formatting" %>
      </td>
    </tr>
    <tr>
      <td valign="bottom" style="padding-bottom:15px;">
       <%= submit_tag I18n.t('txt.views_topics.save_reply', :default => 'Save reply') %>
     </td>
   </tr>
  </table>
  <% end %>

Many thanks.

Answer 1

I think you have two problems here.

1. Keeping validation errors through a redirect
2. Repopulating the form fields so the user doesn't have to enter all the information again.

Both of these things are connected.

Validation errors are usually displayed through the error_msg_for method which acts on an object. Usually provided by the controller as the an instance variable of object that could not be saved. That same instance variable is used to repopulate the form.

During a redirect the controller will usually instantiate an instance variable using the params hash. So any information used to determine why a save failed is lost. Normal resources will render on save failure and redirect on success, this causes two things happen.

1. The instance of the object is passed to error_msg_for creating that nice uniform error box.
2. The instance of the object is used to populate the fields of the form, allowing your user to edit only what is necessary.

I don't know Beast so well, so I'm not sure if the form to create threads is an active record model. But the following will give you an idea how to work around your problem. It would involve modifying your local copy of the Beast plugin, so if you're using a tool to keep it updated, your changes might get lost.

I've been using these following methods to get your validation problems. Assuming the form you're talking about is based on a nmodel they should provide you with everything you need to repopulate a form.

Essentially you store a shallow copy of the object with the errors in the flash hash, using clone_with_errors. You have to use a shallow copy or else you'll run into problems when displaying errors for records with multiple associations.

Then I use my_error_msg_for which takes the same options as the standard error_msg_for to build the error messages html. I only wrote it because for some reason the standard error_msg_for method didn't work with objects stored in the hash. It's almost identical to the official source version of error_msg_for which was troubling.

/app/controllers/examples_controller.rb

class ExamplesController < ApplicationController
  def update
    ...

    if @example.save 
      regular action
    else
      flash[:errors] = clone_with_errors(@example)
      respond_to do |format|
        format.html redirect_to(@example)
      end
    end
end

/app/views/examples/show.html.erb

<div id="error">
        <% if flash[:errors] && !flash[:errors].empty? then -%>

        <p ><%= my_error_msg_for flash[:errors] %></p>

        <% end -%>
</div>
...

Here's the code you need to make it all work.

application_controller.rb

 def clone_with_errors(object)
    clone = object.clone
    object.errors.each{|field,msg| clone.errors.add_to_base(msg)}
    return clone
  end

application_helper.rb

 def _error_msg(*params)

    options = params.extract_options!.symbolize_keys
    if object = options.delete(:object)
      objects = [object].flatten
    else
      objects = params.collect {|object_name| instance_variable_get("@#{object_name}") }.compact
    end
    count   = objects.inject(0) {|sum, this| sum + this.errors.count }
    unless count.zero?
      html = {}
      [:id, :class].each do |key|
        if options.include?(key)
          value = options[key]
          html[key] = value unless value.blank?
        else
          html[key] = 'errorExplanation'
        end
      end
      options[:object_name] ||= params.first
      options[:header_message] = "#{pluralize(count, 'error')} prohibited this #{options[:object_name].to_s.gsub('_', ' ')} from being saved" unless options.include?(:header_message) && !options[:header_messag].nil?
      options[:message] ||= 'There were problems with the following fields:' unless options.include?(:message) && !options[:message].nil?
      error_messages = objects.sum {|this| this.errors.full_messages.map {|msg| content_tag(:li, msg) } }.join

      contents = ''
      contents << content_tag(options[:header_tag] || :h2, options[:header_message]) unless options[:header_message].blank?
      contents << content_tag(:p, options[:message]) unless options[:message].blank?
      contents << content_tag(:ul, error_messages)

      content_tag(:div, contents, html)
    else
                                        ''
    end

  end

  def my_error_msg_for(params)
    _error_msg_test :object_name => params[:object].class.name.gsub(/([a-z])([A-Z])/,'\1 \2').gsub(/_/, " "),
    :object => params[:object], :header_message => params[:header_message], :message => params[:message]
  end

Answer 2

I'm afraid I don't know anything about Beast, but speaking generically, everything is lost when you redirect. It's a new page request, and everything is reset unless it's been stored somewhere (the database or the session, normally.)

The normal flow you tend to see with forms is to redirect if the object is saved, but to render if the save fails. The view file can then pick up whatever variables have been set in the controller - which would normally include the object that didn't save and its validation messages.

Sorry that doesn't solve your problem, but hopefully it may give you some clues.

Answer 3

My answer to a very similar question posted more recently here on StackOverflow covers a number of pros and cons to the redirect_to vs. render debate. I'd love to hear if anyone has any other pros/cons to add to the discussion.