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.