Robby on Rails

While reviewing some code recently, I came across controller code that resembled the following.

if @customer.save
  CustomerMailer.deliver_welcome_message(@customer)
  flash[:message] = "Your account has been successfully created. We've sent you a welcome letter with..."
  redirect_to dashboard_path
else
  ...
end

Fairly typical Rails code. Nothing alarming here, but I wanted to evaluate the call to the mailer in this scenario. When it comes to sending emails from your application, you can choose to do it from the controller as in the example above or in your models. Our team prefers to do this from our model via a callback as we are considering this to be part of our business logic.

Each time a customer is created, we want to send them an email. This can be moved into the model and resembled something like the following..

after_create :send_welcome_message #, other callbacks..

def send_welcome_message
  CustomerMailer.deliver_welcome_message(self)
end

There are a few benefits to doing it this way.

1. We can test that this is being triggered within our model specs instead of our controller specs. (we prefer to spend more of our time working within models than controllers)
2. We remove the dependency that all requests must be processed through our controllers.
   • Example: We may one day create rake tasks that data and want these emails to still be sent out. (We’ve had to do this a few times)

I definitely don’t think doing this via controllers is a bad idea, I just lean towards keeping controllers as dumbed down as possible. This allows us to have less controller code that is focused on passing data to/from models and letting our models do the heavy lifting.

UPDATE: DHH was kind enough to post a more detailed response on his blog.

I was going through an older project of ours and cleaning up some specs and noticed how often we were doing the same thing in several places. When we started the project, we didn’t get the benefits of shared groups. Now that we have some time to go through and update some of our older specs, I’ve been trying to take advantage of the features currently available in RSpec. One feature that I haven’t seen a lot of mention of by people is shared groups, so I thought I’d take a few minutes to write up a quick intro to using it.

To pick some low-hanging fruit, let’s take an all-too-familiar method, which you might be familiar with… login_required. Sound familiar? Have you found yourself stubbing login_required over and over throughout your specs?

describe Admin::DohickiesController, 'index' do

  before( :each ) do
    controller.stub!( :login_required )
    Dohicky.should_receive( :paginate ).and_return( Array.new )
    get :index
  end

 ...
end

If you’re requiring that a user should be logged in when interacting with most of the application (as in the case of an administration section/namespace), you might want to consolidate some of your work into one shared specification group. The basic premise behind this is that you can write a typical describe block and load it into any other spec groups that you need. For example, in our case, we’ll need to stub login_required in several places. We can set this up in one shared group and reference it wherever necessary.

For example, here is what we’ll start off with.

describe "an admin user is signed in" do
  before( :each ) do
    controller.stub!( :login_required )
  end
end

describe Admin::DohickiesController, 'index' do
  ...

However, the new describe block isn’t accessible from the block at the bottom of the example… yet. To do this, we just need to pass the option: :shared => true as you’ll see in the following example.

describe "an admin user is signed in", :shared => true do
  before( :each ) do
    controller.stub!( :login_required )
  end
end

Great, now we can reference it by referring to it with: it_should_behave_like SharedGroupName. In our example above, this would look like:

describe "an admin user is signed in" do
  before( :each ) do
    controller.stub!( :login_required )
  end
end

describe Admin::DohickiesController, 'index' do
  it_should_behave_like "an admin user is signed in"

  before( :each ) do
    Dohicky.should_receive( :paginate ).and_return( Array.new )
    get :index
  end

 ...
end

describe Admin::DohickiesController, 'new' do
  it_should_behave_like "an admin user is signed in"

  before( :each ) do
    @dohicky = mock_model( Dohicky )
    Dohicky.should_receive( :new ).and_return( @dohicky )
    get :new
  end

  ...

That’s it! Pretty simple, eh? We can now reference this shared group in any describe blocks that we want to. A benefit to this approach is that we can make change the authentication system (say, we decide to switch it entirely and/or even just change method names, set any other prerequisites necessary when an admin is signed in), we’ll have a single place to change in our specs. (tip: you can put these in your spec_helper file)

You can learn more about it_should_behave_like and other helpful features on the RSpec documentation site.

If you have any suggestions on better ways of handling things like this, please follow up and share your solutions. I’m always looking to sharpen my tools. :-)

Update

In response, Bryan Helmkamp suggests that a better solution is to define a method in our specs like, for example: build_mock_user_and_login. then calling it in our before(:each). So, maybe the approach above isn’t the most ideal method but I did wantt o draw some attention to it_should_behave_like. I suppose that I need a better example.. another post, perhaps? :-)

Also, Ed Spencer has posted an article titled, DRYing up your CRUD controller RSpecs, which will introduce you mor to it_should_behave_like.

Thanks for feedback people!

Related Posts

• Be Careful that you don’t Stub your Big Toe
• Spec Your Views

If you’re working with Ruby on Rails and are looking for ways to improve your existing code base, I would encourage you all to read the following blog posts.

• Skinny Controller, Fat Model, Jamis Buck
• Find methods in controllers, by Graeme Nelson
• RailsConf Recap: Skinny Controllers, The Rails Way
• Rspec notes from the trenches 2, by Courtenay

Hopefully… you’ve already read each of them and as a result… put your controllers on a diet.