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.

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.

My colleague, Gary, keeps a stack of Ruby and Rails books on his desk and was implementing an Observer into a client project. It appears that the Agile Web Development with Rails book is still encouraging people to do the following in order to load an Observer.

# app/models/flower_observer.rb
class FlowerObserver < ActiveRecord::Observer
  observe Flower

  def after_create(model)
    # model.do_something!
  end
end

# controller(s)
class FlowerController < ApplicationController
  observer :flower_observer
end

What is wrong with this approach?

Well, in order for your Observer to be used, the model(s) callbacks that it is observing need to be triggered through a controller. If you end up writing any scheduled rake tasks, your observer will not be called. In my opinion, the controller shouldn’t know this much about the model. In fact, the model doesn’t even really know about it’s observer… so why should a controller?

This was actually changed a long time ago (I previously blogged about a different solution here) and the Rails docs for ActiveRecord::Observer are currently correct.

Observers in the Environment

If you open up a recent version of config/environment.rb, you notice in the comments the following.

  # Activate observers that should always be running
  # config.active_record.observers = :cacher, :garbage_collector

Take a moment to go ahead and specify which observer(s) you’d like to load into your Rails environment.

config.active_record.observers = :flower_observer

Then you can remove your observer calls in all your controllers, because that’s not where you should be defining them.

Also, if you’re not using Observers yet, I’d really encourage you to consider reading up on them and giving them a try.