diff options
| -rw-r--r-- | perllib/FixMyStreet/App.pm | 9 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Contact.pm | 5 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Email/Sender.pm | 5 | ||||
| -rw-r--r-- | t/email/sender.t | 36 |
4 files changed, 47 insertions, 8 deletions
diff --git a/perllib/FixMyStreet/App.pm b/perllib/FixMyStreet/App.pm index 5575d915e..c1628d010 100644 --- a/perllib/FixMyStreet/App.pm +++ b/perllib/FixMyStreet/App.pm @@ -297,7 +297,7 @@ sub get_override { =head2 send_email - $email_sent = $c->send_email( 'email_template.txt', $extra_stash_values ); + $success = $c->send_email( 'email_template.txt', $extra_stash_values ); Send an email by filling in the given template with values in the stash. @@ -309,6 +309,8 @@ set those fields in the email if they are present. If a 'from' is not specified then the default from the config is used. +Returns the email on success, false on failure. + =cut sub send_email { @@ -353,14 +355,15 @@ sub send_email { my $email = mySociety::Locale::in_gb_locale { FixMyStreet::Email::construct_email($data) }; + my $result = 0; try { FixMyStreet::Email::Sender->send($email, { from => $sender }); + $result = $email; } catch { my $error = $_ || 'unknown error'; $c->log->error("$error"); }; - - return $email; + return $result; } =head2 uri_with diff --git a/perllib/FixMyStreet/App/Controller/Contact.pm b/perllib/FixMyStreet/App/Controller/Contact.pm index 5a077d8c3..e1eb64ba3 100644 --- a/perllib/FixMyStreet/App/Controller/Contact.pm +++ b/perllib/FixMyStreet/App/Controller/Contact.pm @@ -257,10 +257,7 @@ sub send_email : Private { $params->{from} = $from; } - $c->send_email('contact.txt', $params); - - # above is always succesful :( - $c->stash->{success} = 1; + $c->stash->{success} = $c->send_email('contact.txt', $params); return 1; } diff --git a/perllib/FixMyStreet/Email/Sender.pm b/perllib/FixMyStreet/Email/Sender.pm index e6148a56c..2fb819fbc 100644 --- a/perllib/FixMyStreet/Email/Sender.pm +++ b/perllib/FixMyStreet/Email/Sender.pm @@ -28,11 +28,14 @@ sub build_default_transport { if ( FixMyStreet->test_mode ) { Email::Sender::Util->easy_transport(Test => {}); } elsif ( my $smtp_host = FixMyStreet->config('SMTP_SMARTHOST') ) { - my $type = FixMyStreet->config('SMTP_TYPE') || ''; + my $type = lc (FixMyStreet->config('SMTP_TYPE') || ''); my $port = FixMyStreet->config('SMTP_PORT') || ''; my $username = FixMyStreet->config('SMTP_USERNAME') || ''; my $password = FixMyStreet->config('SMTP_PASSWORD') || ''; + die "Bad SMTP_TYPE config: is $type, should be tls, ssl, or blank" + unless $type =~ /^(tls|ssl|)$/; + my $ssl = $type eq 'tls' ? 'starttls' : $type eq 'ssl' ? 'ssl' : ''; my $args = { host => $smtp_host, diff --git a/t/email/sender.t b/t/email/sender.t new file mode 100644 index 000000000..49310db83 --- /dev/null +++ b/t/email/sender.t @@ -0,0 +1,36 @@ +use FixMyStreet::Test; +use FixMyStreet::Email::Sender; +use Test::Exception; + +# Specifically testing live email sending errors +FixMyStreet->test_mode(0); + +subtest 'SMTP settings' => sub { + FixMyStreet::override_config { + SMTP_SMARTHOST => 'localhost', + SMTP_TYPE => 'bad', + }, sub { + throws_ok { FixMyStreet::Email::Sender->send('test') } + qr/Bad SMTP_TYPE config: is bad, should be tls, ssl, or blank/, 'Bad SMTP_TYPE throws'; + }; + + FixMyStreet::override_config { + SMTP_SMARTHOST => 'localhost', + SMTP_TYPE => 'TLS', + }, sub { + throws_ok { FixMyStreet::Email::Sender->send('test') } + qr/no recipients/, 'Upper case SMTP_TYPE passes, no recipients throws'; + }; +}; + +subtest 'sendmail default' => sub { + FixMyStreet::override_config { + SMTP_SMARTHOST => '', + }, sub { + FixMyStreet::Email::Sender->reset_default_transport; + throws_ok { FixMyStreet::Email::Sender->send('test') } + qr/no recipients|couldn't find a sendmail/, 'Sendmail throws some form of error'; + }; +}; + +done_testing(); |