Sending email: Controllers versus Models
81 comments Latest by Hybrid Cars Thu, 18 Mar 2010 03:57:46 GMT
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
...
endFairly 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)
endThere are a few benefits to doing it this way.
- 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)
- 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.
Enjoying the content? Be sure to subscribe to my RSS feed.






I usually send emails from models, but sometimes I like to use an observer to manage all the mail notifications
I think is a bad idea. The purpose of the controller is to route the integration between request, processing, and views. In my mind sending an email is part of the view. It just happens that this view is delivered via SMTP instead of HTTP. It ties the model to this view and to the SMTP protocol, which I think is a muddling of concerns.
I’ll go out an a limb and disagree with DHH. I’d send email from a model if that choice keeps my code dry.
@DHH so how do you deal with background processing emails ?
@DHH I think you can be wrong. For example if your emails are first saved to database and than sent by background process via SMTP than the concept of working with emails as with normal records is something which I think is Business logic related. Therefore is pretty fine to have them in models where I think they should be.
If you are sending emails in before/after hooks of your Rails model you have to always remember this in the future when you are doing migrations. You don’t want to accidentally re-send all of your users an email as a side-effect of your migration.
DHH,
I suspect that my example was a bit basic in nature. What about the following scenario:
If our system is setup to automatically send a notice to Joe each time someone answers their question, should that be done in the controller versus the models?
Another scenario:
Someone submits an article to a site and it’s now has a pending state. An approver later reviews and approves it… moving it to a published state. Our business logic is that when an article moves from pending to published, that the author be notified via email. If we’re doing this with a resful interface with a basic update action in our controller, wouldn’t we end up with a lot of checks to determine if the state changed from X to Y?
Just some different scenarios that we encounter quite often, which we’ve found handling this in our models easier.
The “email as a view” notion has a problem, though, because the email frequently is something totally different than what’s being shown on the user’s screen.
What’s more, the email may very well be going to someone else than the current user.
So it may be a view of something, but unless it’s literally a re-rendering of the same data that’s been handed to the view being rendered by the app, it seems misleading to try to shoehorn it into the “just another view, delivered via SMTP”.
When you have different data, a different destination user, and a different delivery mechanism, seems to me that’s a different action, not a different view.
I will now go and offer up a small rodent as sacrificial penance for disagreeing with DHH, and publicly as well.
@Emilien, background processing is done by async jobs, and (done right) those jobs are controllers, too (just not descendants of ActionController::Base). I’m with DHH: sending email from a model restricts where and how the model can be used, just as rendering a view from the model would.
@Sundog, there are other ways to avoid repeating yourself. If you send the same email from multiple controllers, try refactoring that duplication into a module that those controllers then include. This keeps your code lean, your controllers small, and your models general.
I feel like there’s definitely room to improve accross the board.
There has been some work done on ActionMailer for Rails 3, but I think that it mainly concerns the SMTP internals.
The “view delivered via SMTP” paradigm only holds in so far as building the email, but not so much sending it, which is what is at stake here. A client pulls down a site over HTTP, but an app pushes an email out over SMTP — I don’t believe that the same paradigm can hold for delivering content when it’s push vs. pull.
That said, I’m still always a little concerned with putting emails in the model: emails don’t really belong in the business logic of the model.
It feels like we all need a paradigm shift, a lightbulb moment or a new way to deliver mails in Rails!
@DHH I disagree as well. Sending out Emails will, in many cases, simply be part of “business logic”.
I would agree with you if you would start to assemble email messages in the model (That would feel very muddling). But if your mailer classes (that actually feel a bit controller-ish to me) are written in a sense making fashion, all you pass into the mailer are a few domain objects.
But anyway, since mail stuff usually happens somewhere in between user interaction and pure business logic, this may simply be a case of “do whatever you think will make sense”.
Agreed with most comments here, it’s a different action, not just a different view. You could well be doing something like sending a report to an admin when a master record is updated, or something that has nothing to do with the update action itself. I think observers are more appropriate in most cases.
On a related note, what part of MVC is ActionMailer? I’ve most often seen people put them in app/models, but it seems more like a controller to me.
It seems like a red herring to consider an email to be a view so closely related to those delivered by HTTP.
It’s also inaccurate to suggest that implementing email sending in the model is coupling it with either a particular template, or with SMTP. All we are coupling the model with is the ActionMailer API. The fact that it uses SMTP underneath is completely irrelevant, and could easily be replaced with other delivery mechanisms, as long as the ActionMailer API (or whatever set of methods the model is using to trigger email sending) remains constant.
Personally, I would always keep email concerns out of the controller, and almost always implement them using observers and some out-of-request mechanism. I remain far from convinced that keeping that logic in the controller has any advantages at all.
Robby, both these examples offer no compelling reason to me why they shouldn’t be controller triggered. The views also depend on model state to determine exactly what to render. This is the same thing.
Make your domain model expressive enough that it can encapsulate the answers that controllers and views will pose of it and there will be no violations of DRY.
I like to use callbacks for sending emails for the same reasons. The less going on in your controllers the better :)
This seems like a good time for an observer. Sending the welcome email is neither the User model’s primary business concern nor the controller’s responsibility.
I do this in observers. Any callback that doesn’t have incidence on the model itself (Customer), should be removed from the model class itself to customer_observer, I think
But I do agree it feels more elegant to have it triggered by the model.
James, using observers looking at controller actions and reacting can be a reasonable way to structure your code. But that’s just a way of layering or AOP’ing your controller code. I don’t even think that’s a good idea, but it’s a better idea than having your model interface directly with ActionMailer.
Trevor, I think this is the root of the problem. The whole “skinny controller” idea was so successful that everyone went off to purge any and all code and flow from their controllers. Whether related to the model or not. I don’t think that’s a reasonable path.
Andrew, I agree that having ActionMailer in app/models directory is a mistake and part of the problem here. We should get that updated for Rails 3.
I do this in observers. Any callback that doesn’t have incidence on the model itself (Customer), should be removed from the model class itself to customer_observer, I think
But I do agree it feels more elegant to have it triggered by the model.
I am definitely more on the side of mails sent via models instead of via controllers.
I like to think of a mail as the result of an event.
Most of my application code ends up featuring the states of models with various events raised in the lifetine of the application. Sometimes with a separate event/service layer.
It should be in the model because these events are/can be triggered in several ways, potentially not even HTTP.
Therefore if anything would be useful is a paradigm within rails for states, events and handlers. As I tend to write my own…
When it comes to testing I generally just mock or stub out the occurance of these events when I am not specifically testing them, it makes things quite simple in my mind.
YMMV.
I would prefer the observer route, as it’s outside of your controller logic, and outside of your model logic. The observer could also let you specify sending it to a background job if you so desired. Thoughts on that route?
I would prefer the observer route, as it’s outside of your controller logic, and outside of your model logic. The observer could also let you specify sending it to a background job if you so desired. Thoughts on that route?
I am definitely more on the side of mails sent via models instead of via controllers.
I like to think of a mail as the result of an event.
Most of my application code ends up featuring the states of models with various events raised in the lifetine of the application. Sometimes with a separate event/service layer.
It should be in the model because these events are/can be triggered in several ways, potentially not even HTTP.
Therefore if anything would be useful is a paradigm within rails for states, events and handlers. As I tend to write my own…
When it comes to testing I generally just mock or stub out the occurance of these events when I am not specifically testing them, it makes things quite simple in my mind.
YMMV.
I agree to DHH from a theoretical point of view. But in fact using observers is pretty straight forward.
While we’re on the topic, I’m curious about another scenario (not involving an email notification).
Let’s say that you’re working with SalesForce. After a new customer is created, you need to create a new record in SalesForce. I’d approach this in a similar manner through a callback in the Customer model. We’re sending data to a third-party API and wouldn’t do this within a controller.
I suppose that I’ve always looked at ActionMailer as a closely connected API that we send some data and it takes care of the rest… as if it has it’s own MVC architecture that we interface with.
I was hoping to hear from people who prefer the other Controller-based approach, but don’t see any clear advantages and/or disadvantages to either.
Robby, depends on whether the SalesForce model is part of the data model/integrity of the Rails-side model. If it is, then I would treat that SalesForce model similarly as a has_many or belongs_to relation. In other words, it belongs in the model.
If it’s more akin to a replication/view and the original model will function just fine regardless of whether the SalesForce model is there or not, then I absolutely think this is a controller/observer issue and should not be muddled into the model.
For the record, I think using observers is a reasonable way of AOP’ing your controller if you think the mailer concern is pushing on too much or you can’t come up with another clean way of abstracting the functionality from multiple controllers that all need the same logic.
Even that sounds a bit fishy, though. I’d probably rather go with a module plus a named filter, but it’s certainly a better way than straight in the model.
I think part of the problem is that the call to:
CustomerMailer.deliver_welcome_message(@customer)
it’s that it’s completely encapsulated. It seems to be disjointed from the flow from model -> controller -> view
Looking at it another way, though, DHH’s argument does look a bit stronger.
If you look at the controller as “glue” between models (plural) and views (plural), then it seems only natural that the call to send an email happens at the point at which “models” and “views” are brought together.
... it’s just that … to me it still doesn’t FEEL quite right.
I agree with Robby here. What about a use case where users can optionally receive Email, SMS, or Twitter notifications for an event – would you want that business logic in a controller?
I think both “sides” here are wrong. The real answer of course being “it depends”. ;)
If you’re pretty sure that you always want the email, then use callbacks or observers. If the email is specific to an interaction with the app, then the controller might more sense.
But sometimes what you need is another layer between model and controller, and separate business logic from domain logic.
I normally do that in controllers indeed (if the load allows it).
The controller glues stuff: do this, and send that. For me that fits nicely into the controller logic. An observer is fine as well, but the controller is simpler and (in my view) an appropriate layer.
As a nice side-effect we can pass URLs to the email, so most of my applications have no hostname in the config file. (This is a nice side-effect, not a reason for chosing the layer.)
I’d put this in the controller every time. I used to think (and do) otherwise.
Every time I’ve put email sending in model callbacks, I’ve ended up taking it out. The simplest example is that a User::create during registration might send an email.
However, an administration create form would not.
That’s not to say that code can’t be abstracted into the model but the actual action of sending is a controller concern. For example, I’ll often put the code that adds email sending to a job queue in a model method, making the controller code simple:
@user.queue_notifications
Regardless of opinions on responsibilities, the code going in a controller gives you the most control.
My first impression: I don’t think that’s really DHH.
“Sending an email is part of the view”
That’s nonsense. An email, of course, has a view unto itself, but an email is not inherently related to an HTTP request per se. For every case where you might say “I don’t want this email sent if I create an object from the console” there is a matching case to oppose it. Plus, there are all the emails that are triggered by other vectors (cron jobs, notifications, via API etc).
But as Robby originally said, the given example is pretty trivial, it doesn’t make much difference one way or the other. In order to have such a strong opinion you really should be looking at more complicated use cases, and if you looked at different complicated use cases you’d find different optimal solutions. I don’t think anyone should be getting so religious over such a barebones example.
@Gabe : http://twitter.com/dhh/status/5769040367
Stephen Bartholomew’s argument is the best I’ve seen so far for the controller side of things.
If the email is just another view, it should be part of “render” at the end of a controller action:
Instead of:
render :action => “action_name”
Something like:
render :http => “action_name”, :smtp => “action_mailer_name”
This type of approach would solidify DHH’s argument. Since it doesn’t currently work this way, email does not feel like a view.
These two code snippets express different things: - In the first case we’re saying that when a customer is created in this specific location in the application, send a welcome email. - The second is saying to send a welcome email whenever a customer is created anywhere in the application.
My mental model is email rendering and deliver is an activity that just happens to have a visible aspect that lends itself nicely to the use of templates. Yet that alone does not make it a concern of the controller. Even when coupled with the fact that ActionMailer methods correspond to controller actions, I’m not compelled to put ActionMailer calls in controllers.
A request that is handled by a controller action results in one response. While the controller’s job is to interact with related models and to prepare and present a response, the email is usually not the related model nor the response. In most cases, something else is the response and the email creation is simply one of many activities that occur due to a change in state or value from the controller’s call to the related method in the model (e.g. create, save, destroy).
Using @Stephen Bartholomew’s example, I think it depends on what the requirement is. Is the requirement that when a user is created from the admin form, that an email not go out? Or, is the requirement that when a user is created by and admin that an email not be delivered?
If it’s the later, then putting the logic (unless created_by is an admin…) in the model gives you absolute control. No matter in which what controller User::create is called, the email will not be delivered if the user is created by an admin. Even if someone comes along later and creates a User in a new controller, the business rule will hold even if this new developer was not aware of it.
If the suggested best practice for Rails 3 is to send mail from controllers, then it had better be DRY as well as easy to understand, or else people are going to continue to think of smtp rules as business logic and bury it in the models.
Using module mixins seems like a decent way of keeping this out of the model while keeping controllers DRY, but I have a feeling that it will be easy to confuse people new to ruby and mixins—which pretty much covers everyone I’ve worked with in the last 1 1/2 years. If that’s the case, then ActionMailer documentation will need to include much more explanation on how mixins work, how to test them, where to put them in a Rails project, why you would use them instead of an observer…
I personally like both observers and mixins, but every problem has a solution particular to it. Sometimes you really do need your model to do view-level logic. Sometimes you can’t get around business logic in a view. It all comes down to best practices, and a framework that allows you to put those best practices into use without destroying performance.
This distinction between Model, Controller, Mixin and Observer is just a case of splitting hairs over semantics. There are valid arguments for it to go in either of the three spots. My opinion is that mail delivery belongs in an Observer.
Although sending emails feels like business logic, ActionMailer is written like a controller (despite its subclasses existing in app/models).
What the before/after_save callback really accomplishes is triggering an ActionMailer request. If you treat ActionMailer like any other controller the choice becomes clear.
Placing the code in the model runs contrary to the MVC workflow. Placing it in the controller is not much better. Controllers should not be communicating directly with each other, the best you get is redirection.
Placing it in an observer makes the most sense. Even if there is functionally no difference between declaring the callback in the model or an observer of the model.
@Stephen Bartholomew That can be solved by adding an attr_accessor to your model. When 90% of my creates require an email. I’ll make that the default and throw an unless clause onto the delivery statement and add the extra parameter in the cases I don’t need it.
User.create(:email => "foo@bar.baz", :cancel_delivery => true)I agree with DHH on this.
The email is just another view. You are forgetting about the implicit render Rails’ ActionController does for you. MVC is a bigger pattern than just HTTP, and while you most commonly render a web page as your view, sending an email fits the model just as well.
In my mind I think of the controller (in MVC) as the traffic cop. It says what the models should do, and then it says what the output should be.
@bryanl: When the traffic cop says the output should be both an email render and an action redirect (as in Robby’s original example) the mental model of ActionController doing one or the other falls apart.
man, that is kind of weird. i definitely don’t agree with putting business rules in domain objects – call me old school, but just because you can do these things doesn’t mean that it is right.
to me, it seems that this type of behavior should be abstracted in a “service” that accepts a message or a signature of items to be detailed in the email. so your business rule should be able to “consume” this service several different ways for different types of messages in multiple business rules across the application
rails gives you the ability to build something like this – it’s generally called a controller. i think of a controller as a “service interface” or a “service endpoint” with some orchestration or business rules sitting underneath it which interact and use and return data via its domain objects (or models).
call me krazee, but that seems a bit more tangible, usable, declarative, etc. to me.
holla at your boy!!!
My take on this:
- many times there is no one-to-one relationship between the http request and sending emails: Depending on the model state I sometimes may want to send no email, one email or five emails to one or maybe a couple of different recipients -> to me that is part of the business logic -> those emails should be sent by models
- I think there is really no difference in sending emails, SMS notifications, pushing some sort of message to a MQ, dropping tweets or maybe consuming another 3rd party web service. Again I consider this to be business logic -> code that belongs to the models
- I think of sending out an email as requesting a special service to send out that email. I see this as a new request besides the (pending) original http request. The model code, which triggers the email, acts as client, which requests an email service to send out an email. Now the server-side of this service is actually ActionMailer. So it makes sense that ActionMailer is treated like a controller and is located in the controller folder. It’s a service offered by Rails in-process style. ActionMailer simply responds to the client’s request in another request/response cycle.
Bottom line: Triggering emails should be done in models, ActionMailer is the controller part of the email service offered by Rails and belongs to the controller folder.
PS: Another nice side effect when sending emails in models when you store the mails in the DB in the first place: If a rollback happens the email is gracefully rolled back together with all your other database changes.
I can think of situations where I would send an email from a model even though I haven’t done it all that often. This, as in many other cases becomes a matter of taste.
But one case that hasn’t been discussed much is mass mailing. Suppose I need to allow a user to invite all his friends to an event. This is getting more and more common these days. Nowadays every client seems to want Facebook -type functionalities.
Me and a few guys at TWG have been working on this mass mailing problem for a while now. We got tired of trying to figure out this same problem over and over so we decided to create a web service called PostageApp.com that allows us to use an API call to say: “Here is the content of my email, here is the list of recipients. Go ahead and send it.”
This moves the work into my app’s controller where I think it should be. Also, it allows you to send out mass mailings with one fast call and avoids locking up the app. That’s another problem with sending email – it’s a synchronous task. Model or controller, it’s still going to slow down your user experience.
I think it’s kind of crazy that I have to create extra tables and cron jobs just to let my client send a few emails at a time (see AR_mailer).
We’re using PostageApp.com on almost all our projects and it has made our lives a lot easier. It’s basically our attempt at solving the dual personalities that email has in Rails.
Sending emails after an update or create event has come back to haunt me every single time. Eventually you will have that batch process or data migration or otherwise that needs to touch records without dispatching a million emails and it will be impossible to make an exception.
The decision whether to trigger a mail or not often depends on the context. Controllers have that context, so they should decide.
@Henning
This is how I tell whether or not a record was simply touched or not:
self.changed.size 1 and self.changed[0] “updated_at”
The UGG Knightsbridge has a slimmer wholesale ugg boots leg than mode and great looks. Originally intended to be damaged by storm. The UGG Knightsbridge wader is one of UGG’s tall boots. discount ugg boots The drudgery toed box and well-known women in the world. Take one look at UGG boots and you can keep an infant baby comfortable, just picture how good it feels on feet of some of UGG’s most discount ugg cardy boots accepted and boxy heel is right in vogue styles of your feet. The UGG Knightsbridge wader is no different, made with skirts, sundresses and other leg showoffs. classic,
Cool! Thanks for compiling these codes. I just started using cucumber and I’m digging it a lot.
I like your website, It has been a pleasure to read the different articles in it, I’m subscribing to your rrs feed right now!
I like your website, It has been a pleasure to read the different articles in it, I’m subscribing to your rrs feed right now!
I like your website, It has been a pleasure to read the different articles in it, I’m subscribing to your rrs feed right now!
Very nice looking post, as well as the rest of the site. Really does help me with Ruby on Rails.
Thanks! :)
Very good post I enjoy your blog keep up the great posts
Thanks for sharing this post. This is a very helpful and informative material. Good post and keep it up. Websites are always helpful in one way or the other, that’s cool stuff, anyways, a good way to get started to renovate your dreams into the world of reality.
The artcle is very helpful!
I’ll go out an a limb and disagree with DHH. I’d send email from a model if that choice keeps my code dry.
I’ll go out an a limb and disagree with DHH. I’d send email from a model if that choice keeps my code dry.
You are really great!
nike shox ghd hair straightener china wholesale
Nice post,thanks a lot.
Very good example and unique idea. I will definate try this method.
Cool content. Check out my informative website about waterproofing a roof.
This is a tremendous article, im thankful I came across this. Ill be back again in the future to check out other posts that you have on your blog.
Cool content. Check out my informative website about waterproofing a roof.
Interesting code programming debate, salute to all of you
Interesting code programming debate, salute to all of you
I wanted to thank you for this great read!! I definitely enjoying every little bit of it I have you bookmarked to check out new stuff you post.
Awesome blog. I enjoyed reading your articles. This is truly a great read for me. I have bookmarked it and I am looking forward to reading new articles. Keep up the good work!
So glad this internet thing works and this article helped. Might take you up on the home advice sometime. Perhaps a guest appearance here?
This is a really good read for me, Must admit that you are one of the best bloggers I ever saw.Thanks for posting this informative article.
I really appreciate the kind of topics you post here. Thanks for sharing us a great information that is actually helpful. Good day!
Thanks for sharing. Post like this offers great benefit. Thank you!
Hope this will make easy sending emails. I will try in server !
The information and tips in this post seems helpful to the Internet marketers on email marketing management. Those who apply your technique will certainly improve their email marketing campaign effectiveness.
As is see it It seems like a red herring to consider an email to be a view so closely related to those delivered by HTTP but maybe i am wrong about this issue
Nice post
ed hardy swimwear ed hardy clothing ed hardy clothes ed hardy t shirts ed hardy jeans
Thanks for sharing.
ed hardy clothing
wholesale ed hardy
ed hardy boots
ed hardy jeans
Hi, I just love the way you integrated the MVC paradigm. Software design in its best implementation.
girl games and even pacman and frogger are there…
Thanks so much for this post! :)