[rt-devel] Move Action::Notify::SetRecipients to Action::SendEmail?

[Jim Meyer]

Howdy!

I'll take 'em both at once.

On Tue, 2002-02-19 at 20:58, Jesse Vincent wrote:
> Autoreply is a one-trick pony. What do you stand to gain by adding
> the layer of indirection?

AutoreplyToRequestorsAndCcs.

[Pardon me while I gush: it's outrageously hip that RT is so well
designed -- that it's architecture is so easy to grasp -- that I could
make a sensible reply that concise. I love good engineering. =]

On Tue, 2002-02-19 at 21:22, Jesse Vincent wrote:
> 
> I should also point out that by default, Notify strips the sender out of the
> message's recipients. The whole reason Autoreply was created as a seperate
> module was to avoid that behavior.

Sorry, I overlooked that difference. 

I think I'd then change my opinion and consider elevating
Notify::SetRecipients() into SendEmail once SetRecipient can be told to
leave the sender in (though not by default) and the behavior if provided
no arguments is to simply return().

That way, Notify can call it and still get the expected behavior,
Autoreply can call it requesting that the sender not be stripped, and
anything which relies on the current behavior of the dummy
SetRecipients() won't be offended.

And again, I'd offer to do the work on this.

Cheers!

--j

> > On Tue, Feb 19, 2002 at 08:51:45PM -0800, Jim Meyer wrote:
> > > Howdy!
> > > 
> > > The subject mostly says it all. 
> > > 
> > > RT::Action::Autoreply derives from RT::Action::SendEmail, in which
> > > SetRecipients() is a mostly empty placeholder waiting to be overridden.
> > > As you might expect, RT:A:AR overrides it by forcing the requestors into
> > > the To field and moving on.
> > > 
> > > Contrast this with RT::Action::Notify, which implements a lovely
> > > SetRecipients() accepting as an argument a comma-separated list any or
> > > all of "Owner, Requestor, AdminCc, Cc or All". This allows RT:A:N to be
> > > used all the different non-"AsComment" scrips just by changing the
> > > arguments.
> > > 
> > > It feels like Autoreply should either derive from Notify (as
> > > NotifyAsComment does) or Notify::SetRecipients() should be promoted to
> > > SendEmail. Oddly enough, I favor the latter because an autoreply is just
> > > a specialized notification (with the proper queue return address set).
> > > 
> > > Any thoughts? I'll happily submit either patch.
-- 
Jim Meyer, Geek At Large                              purp at wildbrain.com