RESTful contact forms

Posted by Matt Jankowski

Apr 04

Just like static content pages – simple “contact form” pages are another thing that can very easily fall into the RESTful pattern if you want them to. And, in the same way that seeing implicit actions in static page controllers keeps me up at night, so does seeing something like this in a controller…

1
2
3
4
5
6
7
8
9
10
11
12
13


class SiteController < ApplicationController

  def contact
    if request.post?
      # Send some mail
      # Set a flash, and redirect back somewhere
    else
      # Render a contact form
    end
  end

end

There are least three things wrong with this small code fragment:

So how do we want to change this? Well, what’s the resource here? I’m going to say the resource is “the contact” or “a contacting event” maybe. I suppose you could go with “Message” as well, or some other naming that reflects the passing of information from a site user back to the team running the site. That deals with the first problem.

We can deal with the action naming problem and the “if request.post?” problem at the same time. In terms of action naming, I think the form we show the user to submit the information should be “new” (although I’ve entertained fascinating arguments for why it should be “show”!), and the action which actually “creates the contact” (ie, sends an email back to the team, or logs the info in the db maybe?) should be “create”. Both are standard RESTful named actions, so that requirement is met.

We end up with a controller like this…

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24


class ContactController < ApplicationController

  before_filter :check_message, :only => :create

  def new
  end

  def create
    Notifier.deliver_contact_form params[:name], params[:email], params[:message], params[:referer]
    flash[:success] = 'Feedback sent successfully'
    redirect_to root_url
  end

  protected

  def check_message
    if params[:message].blank?
      flash.now[:failure] = 'Please supply a message'
      render :action => :new and return false
    end
  end

end

(Note the correct flash.now usage in the before_filter. Again, without that in place, I’d lose sleep.)

...and a new route, like this…

1
2


  map.resource :contact, :controller => 'contact'

(Note the correct singleton resource usage. If I didn’t do that I’d once again lose more sleep and this refactoring wouldn’t be even be worth it anymore because I’d be up all night.)

I like that much better – and as a side effect, I’m going to be able to write separate functional tests for #new and #create, which should clean up the tests a bit as well.

Does anyone else do “RESTful contact forming”? Any better ideas for this simple problem?

Comments on this post

Dan Croak

Apr 04

Dan Croak said,

That request.post? makes me feel dirty.

Dan Kubb

Apr 04

Dan Kubb said,

I don’t have any huge improvements over what you’ve done here, but you could always pass the form values into Notifier.deliver_contact_form using Hash#values_at: <filter:code> Notifier.deliver_contact_form *params.values_at(:name, :email, :message, :referer) </filter>

I’m also not sure the “and return” in the action is necessary. I suppose its an ok idiom to use when you need to return from the middle of the action, but when its the last statement in the action its unecessary.

Randy

Apr 04

Randy said,

It seems like almost all customers want this data in the db as well at some point. Why not just have it be a normal resource with a public new/create and then full control from the admin side? It’s easy enough to do.

Matt Jankowski

Apr 04

Matt Jankowski said,

Dan – yeah, the “and return” was definitely not needed there. I bet that’s left over from the initial refactoring – I updated the post to reflect.

Raghu

Apr 04

Raghu said,

I am guilty of more than one such non-RESTful controller/action. This is helpful – thank you.

How do you suggest I approach the reporting side of things – there might be a several varied reports which have really no mapping to CRUD. They are all essentially “show”s with very different where/find conditions. Is it ok to have a reports_controller and have non-standard actions? Like hot/popular/recent etc etc

Daniel Neighman

Apr 05

Daniel Neighman said,

Just for something completely different. I’ve hacked out a little tool to do simple contact us style forms for any site, dynamic or static. It’s actually just gone live today :)

It won’t put the data in the db but if all your after is a form that gets emailed to you, but as an alternative it may just suite your needs.

If you interested it’s at http://postb.ag

JGeiger

Apr 05

JGeiger said,

I create a Feedback model and then just do the normal REST controller deal on it. The upside as said before in here, it’s saved in the DB.

serge

Apr 05

serge said,



params[:message].blank?

validation on the controller!? ewwhh! ;)

the contact data should be wrapped in a model, there you can validate all you want.

DerGuteMoritz

Apr 06

DerGuteMoritz said,

Just a little thing: You don’t have to return false from a filter in Rails 2.0 anymore to stop the action from being called, rendering or redirecting is sufficient. Yay for having to write less code ;)

Neil Wilson

Apr 07

Neil Wilson said,

That before filter has a bad smell. It is not a cross cutting concern so it shouldn’t be there. If it is ‘only create’ then it should be in create, not hanging around somewhere else so you can give yourself the impression of avoiding a branch statement.

it would be a lot clearer if create said

1
2
3
4
5
6
7


@notifier = Notifier.new params
if has_message?
  deliver_message
else
  ask_for_a_message
end

Which is actually what you are trying to do. Do that and you’ll probably find that some of the local methods delegate nicely to the Model.

Sorry, comments are closed for this article.

© 2000 - 2009 by thoughtbot, inc.
written by a bushel of tiny robots