Best Practices Question

Should models call action mailers, or should those calls always originate from controllers? For example, should user.forgot_password send the email, or should the user_controller.forgot_password?

Just looking for some opinions...

Thanks, Tom

Quoting TomRossi7 <tom@themolehill.com>:

Should models call action mailers, or should those calls always originate from controllers? For example, should user.forgot_password send the email, or should the user_controller.forgot_password?

Unless the action changes the state of a user or requires details not available through the public API of the user model, I'd put it in the controller. It is invoked by a HTML request and should (in my mind) go with the rest the HTML request logic. Unless of course it is being invoked by a cronjob or similar non-HTML request action/event.

Just my $0.02USD,   Jeffrey

With the caveat that I'm exactly two days into learning Ruby/Rails, I'd offer this from a purely MVC perspective. The sending of email is application functionality. The model is for business logic and data manipulation.

When I think about situations like this, I consider an API. If I were developing an API that allowed a user to reset their password, would I want my code to send an email? My answer is "probably not". I'd want the application that _calls_ the API to send that email - if the developer chose to do so. That tells me that the model probably isn't the right place.

Hope that helps.

Rob Wilkerson wrote:

With the caveat that I'm exactly two days into learning Ruby/Rails, I'd offer this from a purely MVC perspective. The sending of email is application functionality. The model is for business logic and data manipulation.

When I think about situations like this, I consider an API. If I were developing an API that allowed a user to reset their password, would I want my code to send an email? My answer is "probably not". I'd want the application that _calls_ the API to send that email - if the developer chose to do so. That tells me that the model probably isn't the right place.

You just said "Dependency Inversion Principle". Look that up.

In almost every case that I've crossed this bridge, I've moved this logic down into models. I

Now you can see why I posted this for discussion. Two voices in my head (1) put it in the model and use dumb controllers (2) emails are related to user interaction logic and belong in the controller.

tomrossi7 wrote:

Now you can see why I posted this for discussion. Two voices in my head (1) put it in the model and use dumb controllers (2) emails are related to user interaction logic and belong in the controller.

Which is easier to test?

I'm not sure there is a significant difference. They are both easily tested.

Note that Rails has a subtle way of stating the framework's position on this: mailers are (by default) generated in app/models. Just sayin'.

--Matt Jones

Matt Jones wrote:

Note that Rails has a subtle way of stating the framework's position on this: mailers are (by default) generated in app/models. Just sayin'.

And that's completely irrelevant to the question at hand. Of course an ActionMailer object is a model, but the OP was asking whether its deliver_* methods should be called from a controller or from another model.

--Matt Jones

Best,

Hi --

Should models call action mailers, or should those calls always originate from controllers? For example, should user.forgot_password send the email, or should the user_controller.forgot_password?

Just looking for some opinions...

I keep the ActionMailer model files themselves in models/, where they land automatically, but then allow the controller to talk to those models as it would to any other. So I'll put this in a controller:

   if params[:forgot_password]      UserNotifier.deliver_pw_help(@user)    else...

and so on. I don't think it's necessary to wrap it in another layer of modeling. Controllers are allowed to be one step away from model logic, and the methods to deliver are already built into the ActionMailer models.

David

Hi --

tomrossi7 wrote:

Now you can see why I posted this for discussion. Two voices in my head (1) put it in the model and use dumb controllers (2) emails are related to user interaction logic and belong in the controller.

Which is easier to test?

I'm very wary of using ease of testing as a criterion for this kind of application design decision. Ease of testing generally has to do with the state of testing tools, as well as some potentially very divergent views on what makes for "easy" testing.

In the case of the ActionMailer question, I don't think either is easier. In both scenarios the test mode of ActionMailer will append the message to AM::Base.deliveries, where it can be examined. Or you can just write unit tests for the mailer itself, and then use a mock mailer in the controller and/or User model tests.

David

Ilan Berci wrote:

While not knowing your problem domain at all, I have a preference for thin dumb controllers (that fit easily within REST), I would move the mailer down into models/ or lib/

hth ilan

+1 (FWIW!)

Views should be dumb, Controllers thin, and Models fat.

If the deliver_ call were one line, it qualifies for the Controller. But...

The delivered template is itself a View. Hence it should be dumb. Hence anything powering it belongs in the _Model_, where fat things belong.

Opinions opinions, oh my!

As I guess has been the general theme of this thread, it really depends on the context in which the mailing is taking place. If its as simple as user clicking a button on your site and in turn some mail goes out, that's controller domain. But if the mailing is part of some more complex logic (simple example: user signs up; requested name needs to be checked, perhaps an invite token counter needs to be decremented, maybe you'll queue this ish etc, and the email goes out at the end), then you'll call the mailer in the part of the model that handles that processing. Send an emailing is a *general enough activity* that there are no rules that say where you should do it in the MVC context.

Hi --

Ilan Berci wrote:

While not knowing your problem domain at all, I have a preference for thin dumb controllers (that fit easily within REST), I would move the mailer down into models/ or lib/

hth ilan

+1 (FWIW!)

Views should be dumb, Controllers thin, and Models fat.

If the deliver_ call were one line, it qualifies for the Controller. But...

The delivered template is itself a View. Hence it should be dumb. Hence anything powering it belongs in the _Model_, where fat things belong.

-- Phlip

Opinions opinions, oh my!

As I guess has been the general theme of this thread, it really depends on the context in which the mailing is taking place. If its as simple as user clicking a button on your site and in turn some mail goes out, that's controller domain. But if the mailing is part of some more complex logic (simple example: user signs up; requested name needs to be checked, perhaps an invite token counter needs to be decremented, maybe you'll queue this ish etc, and the email goes out at the end), then you'll call the mailer in the part of the model that handles that processing. Send an emailing is a *general enough activity* that there are no rules that say where you should do it in the MVC context.

I agree, and I think part of the answer too is that ActionMailer *already* deals with these questions.

Controllers are allowed to make use of the models' APIs. Having a fat model philosophy doesn't mean you can't do this:

   Thing.find(params[:id])

in the controller. Similarly, given an ActionMailer model, there's no reason at all not to use its API in the controller.

If there's more complexity to the mailing -- for example, if it has to search and filter for particular users before it emails them -- then *that* logic can be put in a model. In other words, rather than this in a controller:

   User.find(:all, :conditions => ["staff = ?", true]).each do |staff|      Notifier.deliver_whatever(staff)    end

you would certainly push both the specialized find operation and the delivery into a model, so the controller could just do this:

   User.mail_to_staff

or

   Notifier.notify_staff

But in the simple case, where all you need to do is call a single class method of ActionMailer, there's no reason not to just call it from the controller.

David

Hi --

Ilan Berci wrote:

While not knowing your problem domain at all, I have a preference for thin dumb controllers (that fit easily within REST), I would move the mailer down into models/ or lib/

hth ilan

+1 (FWIW!)

Views should be dumb, Controllers thin, and Models fat.

If the deliver_ call were one line, it qualifies for the Controller. But...

The delivered template is itself a View. Hence it should be dumb. Hence anything powering it belongs in the _Model_, where fat things belong.

That's exactly what ActionMailer does: it keeps the fine-grained stuff in the model, and allows you to control it from the controller using high-level commands (class methods).

I'm becoming very skeptical about the whole thin/fat thing. It's a sound principle, and it's served an important purpose, but as it's purely quantitative, it doesn't provide much real technical guidance. I'm starting to think more along the lines of "shallow" and "deep". The controller has shallow interaction with the model classes and objects; those classes and objects have a dimension of depth into the database and so forth. I'll mull it over and report back :slight_smile:

David

Consider the questions this way: Are there occasions when models should call models directly? Should all model interaction only be mediated by an external controller?

The answers are clearly Yes in the first instance and No in the second. Consider the entire idea of associations for just one example of model to model calls without discrete controllers being necessary or desirable.

The names that you have given your examples somewhat muddy the situation, however. #forgot_password is, in my opinion, a controller type action since it handles an external state, only the user can say whether or not they have forgotten their password, which only the user can communicate to the application. However, #email_password is more likely than not a model method since the email address and password are bound to the user instance regardless of whether the user has forgotten their password or not. The fact that #email_password is an action rather than simply state is besides the point. Models may have lots of actions.

In the first case one is specifying when to act, whilst in the second what to do. "When" is a question I believe best answered in a controller, whether interactive or batch. "What" is a question that really is bound to the model that provides it. So, in my conceptualisation of this situation, users_controller.forgot_password would look something like this:

def forgot_password   @current_user.email_password end

Now, as for the matter of ALWAYS. This issue rather turns on the nature of the request I should think. If one is speaking of a single instance, as in the case of @current_user, then the approach taken above seems suitable. But, what of the case when we are dealing with collections?

Say the issue was who to transmit a personalised newsletter to, given a set of criteria. In this case, does it make sense to use a model method to send the email or should a controller mediate between the set of qualified addressees and the message to send? I should think that an external controller seems more appropriate here, as the implementation of such a feature in the model would in all likelihood require a class method that would probably start to look an awful lot like a controller method.

IMHO of course.