[Catalyst-dev] Catalyst::View::Email patch courtesy of the one and only.

[J. Shirley]

On 7/27/07, Evan Carroll <lists at evancarroll.com> wrote:
> added verbosity to error to reflect nested args ie AoH throwing off error check
>
> fixed typo in docs, mailer_args is supposed to be an array not a hash,
> now yaml shows it.
>
> removed old * for ->sender->{mailer_args}
> we now support the documented fashion!! but we carp - because it isn't
> supported by Email::Send ie we now coerce into ArrayRef is HashRef is
> supplied.
>
> We also croak if neither HashRef nor ArrayRef is supplied
>
> We now check that Host is supplied if the ->mailer is SMTP
>
>
> diff --git a/Email.pm b/Email.pm
> index e4be27c..b422e54 100644
> --- a/Email.pm
> +++ b/Email.pm
> @@ -35,9 +35,12 @@ In your app configuration (example in L<YAML>):
>              method:     SMTP
>              # mailer_args is passed directly into Email::Send
>              mailer_args:
> -                - Host:       smtp.example.com
> -                - username:   username
> -                - password:   password
> +                - Host
> +                - smtp.example.com
> +                - username
> +                - mySMPTUsername
> +                - password
> +                - mySMTPPassword
>
>  =cut
>
> @@ -110,24 +113,64 @@ Then, Catalyst::View::Email will forward to your
> View::TT by default.
>
>  sub new {
>      my $self = shift->next::method(@_);
> -
>      my ( $c, $arguments ) = @_;
>
>      my $mailer = Email::Send->new;
>
> -    if ( my $method = $self->sender->{method} ) {
> -        croak "$method is not supported, see Email::Send"
> -            unless $mailer->mailer_available($method);
> -        $mailer->mailer($method);
> -    } else {
> -        # Default case, run through the most likely options first.
> -        for ( qw/SMTP Sendmail Qmail/ ) {
> -            $mailer->mailer($_) and last if $mailer->mailer_available($_);
> +    my $chosen_method;
> +
> +    ##
> +    ##  If mailer is provided - check to make sure it is avail.
> +    ##
> +    if ( $chosen_method = $self->sender->{method} ) {
> +        croak "$chosen_method is not supported, see Email::Send"
> +            unless $mailer->mailer_available($chosen_method)
> +        ;
> +        $mailer->mailer($chosen_method);
> +    }
> +
> +    ##
> +    ## Default case, run through the most likely options first.
> +    ##
> +    else {
> +        for my $default_method ( qw/SMTP Sendmail Qmail/ ) {
> +            if ( $mailer->available($default_method) ) {
> +              $mailer->mailer($default_method);
> +              carp "Warning making assumption that $default_method is
> the desired method";
> +              last;
> +            }
>          }
>      }
>
> -    if ( $self->sender->{mailer_args} ) {
> -        $mailer->mailer_args($self->sender->{mailer_args});
> +    ##
> +    ## Runtime checks to make sure that an Array is provided if anything
> +    ##
> +    my $mailer_args = $self->sender->{mailer_args};
> +    if ( $mailer_args ) {
> +
> +      if ( ref $mailer_args eq 'HASH' ) {
> +        carp 'Attempting to coerce HashRef ->{mailer_args} '
> +          . 'into Email::Send required ArrayRef'
> +        ;
> +        $mailer_args = [%{$mailer_args}];
> +      }
> +      elsif ( ref $mailer_args ne 'ARRAY' ) {
> +        croak 'Please send something non-retarded for ->sender->{mailer_args}';
> +      }
> +
> +      ##
> +      ## Further checks here against thine ->{mailer_args}
> +      ## Total bull shit Email::Send does not croak without this
> +      ##
> +      my %hash_args = ( @$mailer_args );
> +      croak 'No ->{Host} provided with method SMTP, '
> +        . 'check to make sure data structure sent to ->{mailer_args}
> is not nested'
> +        if $chosen_method eq 'SMTP'
> +        && !defined $hash_args{Host}
> +      ;
> +
> +      $mailer->mailer_args( $mailer_args );
> +
>      }
>
>      $self->mailer($mailer);
>
>
>
> I am awesome.
>

Your style changes are not going to be accepted.

Adding in the type check on mailer_args is a good idea, and is implemented.

The croak on Host not being specified is just plain dumb.  That
doesn't belong here, that belongs in Email::Send::SMTP, and they
default to 'localhost'.  This is pretty common (although undocumented)
behavior.  I'd recommend you file a ticket against Email::Send::SMTP
asking them to document or change behavior to suit your desires.

I'm not going to change their behavior, since I do specifically state
that Catalyst::View::Email is a gateway to Email::Send.  I will,
however, point it out in the Pod.

-Jay

-- 
J. Shirley :: jshirley at gmail.com :: Killing two stones with one bird...
http://www.toeat.com