Use same handling for cron and non-cron email.

This means that e.g. SMTP authentication is used when set up by all
emails, not just non-cron ones. Fixes #988.

13 files changed, 62 insertions, 53 deletions

diff --git a/perllib/FixMyStreet/App.pm b/perllib/FixMyStreet/App.pm
index e5e483937..fd70ffda8 100644
--- a/perllib/FixMyStreet/App.pm
+++ b/perllib/FixMyStreet/App.pm
@@ -10,7 +10,6 @@ use FixMyStreet;
 use FixMyStreet::Cobrand;
 use Memcached;
 use mySociety::Email;
-use mySociety::EmailUtil;
 use mySociety::Random qw(random_bytes);
 use FixMyStreet::Map;
 
@@ -348,9 +347,19 @@ sub send_email {
 }
 
 sub send_email_cron {
-    my ( $c, $params, $env_from, $env_to, $nomail, $cobrand, $lang_code ) = @_;
-
-    return 1 if $c->is_abuser( $env_to );
+    my ( $c, $params, $env_from, $nomail, $cobrand, $lang_code ) = @_;
+
+    my $first_to;
+    if (ref($params->{To}) eq 'ARRAY') {
+        if (ref($params->{To}[0]) eq 'ARRAY') {
+            $first_to = $params->{To}[0][0];
+        } else {
+            $first_to = $params->{To}[0];
+        }
+    } else {
+        $first_to = $params->{To};
+    }
+    return 1 if $c->is_abuser($first_to);
 
     $params->{'Message-ID'} = sprintf('<fms-cron-%s-%s@%s>', time(),
         unpack('h*', random_bytes(5, 1)), FixMyStreet->config('EMAIL_DOMAIN')
@@ -387,15 +396,16 @@ sub send_email_cron {
     $params->{_line_indent} = '';
     my $email = mySociety::Locale::in_gb_locale { mySociety::Email::construct_email($params) };
 
-    if ( FixMyStreet->test_mode ) {
-        my $sender = Email::Send->new({ mailer => 'Test' });
-        $sender->send( $email );
-        return 0;
-    } elsif (!$nomail) {
-        return mySociety::EmailUtil::send_email( $email, $env_from, @$env_to );
-    } else {
+    if ($nomail) {
         print $email;
         return 1; # Failure
+    } else {
+        my %model_args;
+        if (!FixMyStreet->test_mode && $env_from eq FixMyStreet->config('CONTACT_EMAIL')) {
+            $model_args{mailer} = 'FixMyStreet::EmailSend::ContactEmail';
+        }
+        my $result = $c->model('EmailSend', %model_args)->send($email);
+        return $result ? 0 : 1;
     }
 }
 
diff --git a/perllib/FixMyStreet/App/Controller/Contact.pm b/perllib/FixMyStreet/App/Controller/Contact.pm
index 5a51c8494..f48518d77 100644
--- a/perllib/FixMyStreet/App/Controller/Contact.pm
+++ b/perllib/FixMyStreet/App/Controller/Contact.pm
@@ -4,6 +4,8 @@ use namespace::autoclean;
 
 BEGIN { extends 'Catalyst::Controller'; }
 
+use mySociety::EmailUtil;
+
 =head1 NAME
 
 FixMyStreet::App::Controller::Contact - Catalyst Controller
diff --git a/perllib/FixMyStreet/App/Model/EmailSend.pm b/perllib/FixMyStreet/App/Model/EmailSend.pm
index 7f130c26d..475026267 100644
--- a/perllib/FixMyStreet/App/Model/EmailSend.pm
+++ b/perllib/FixMyStreet/App/Model/EmailSend.pm
@@ -1,5 +1,5 @@
 package FixMyStreet::App::Model::EmailSend;
-use base 'Catalyst::Model::Adaptor';
+use base 'Catalyst::Model::Factory';
 
 use strict;
 use warnings;
@@ -54,7 +54,7 @@ elsif ( my $smtp_host = FixMyStreet->config('SMTP_SMARTHOST') ) {
     push @$mailer_args, username => $username, password => $password
         if $username && $password;
     $args = {
-        mailer      => 'FixMyStreet::EmailSend',
+        mailer      => 'FixMyStreet::EmailSend::DoNotReply',
         mailer_args => $mailer_args,
     };
 }
diff --git a/perllib/FixMyStreet/DB/ResultSet/AlertType.pm b/perllib/FixMyStreet/DB/ResultSet/AlertType.pm
index 5bed95811..b704fa7dd 100644
--- a/perllib/FixMyStreet/DB/ResultSet/AlertType.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/AlertType.pm
@@ -5,7 +5,6 @@ use strict;
 use warnings;
 
 use mySociety::DBHandle qw(dbh);
-use mySociety::EmailUtil;
 use mySociety::Gaze;
 use mySociety::Locale;
 use mySociety::MaPit;
@@ -262,13 +261,12 @@ sub _send_aggregated_alert_email(%) {
             To => $data{alert_email},
         },
         $sender,
-        [ $data{alert_email} ],
         0,
         $cobrand,
         $data{lang}
     );
 
-    if ($result == mySociety::EmailUtil::EMAIL_SUCCESS) {
+    unless ($result) {
         $token->insert();
     } else {
         print "Failed to send alert $data{alert_id}!";
diff --git a/perllib/FixMyStreet/DB/ResultSet/Problem.pm b/perllib/FixMyStreet/DB/ResultSet/Problem.pm
index d3c016be6..a84a309ee 100644
--- a/perllib/FixMyStreet/DB/ResultSet/Problem.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/Problem.pm
@@ -8,7 +8,6 @@ use CronFns;
 
 use Utils;
 use mySociety::Config;
-use mySociety::EmailUtil;
 use mySociety::MaPit;
 
 use FixMyStreet::App;
@@ -438,7 +437,7 @@ sub send_reports {
             }
         }
 
-        if ($result == mySociety::EmailUtil::EMAIL_SUCCESS) {
+        unless ($result) {
             $row->update( {
                 whensent => \'ms_current_timestamp()',
                 lastupdate => \'ms_current_timestamp()',
@@ -501,7 +500,7 @@ sub _send_report_sent_email {
 
     my $template = FixMyStreet->get_email_template($row->cobrand, $row->lang, 'confirm_report_sent.txt');
 
-    my $result = FixMyStreet::App->send_email_cron(
+    FixMyStreet::App->send_email_cron(
         {
             _template_ => $template,
             _parameters_ => $h,
@@ -509,7 +508,6 @@ sub _send_report_sent_email {
             From => mySociety::Config::get('CONTACT_EMAIL'),
         },
         mySociety::Config::get('CONTACT_EMAIL'),
-        [ $row->user->email ],
         $nomail,
         $cobrand
     );
diff --git a/perllib/FixMyStreet/DB/ResultSet/Questionnaire.pm b/perllib/FixMyStreet/DB/ResultSet/Questionnaire.pm
index b7af9e60e..646967bfc 100644
--- a/perllib/FixMyStreet/DB/ResultSet/Questionnaire.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/Questionnaire.pm
@@ -5,7 +5,6 @@ use strict;
 use warnings;
 use Encode;
 use Utils;
-use mySociety::EmailUtil;
 
 sub send_questionnaires {
     my ( $rs, $params ) = @_;
@@ -103,11 +102,10 @@ sub send_questionnaires_period {
                 From => [ $sender, $sender_name ],
             },
             $sender,
-            [ $row->user->email ],
             $params->{nomail},
             $cobrand
         );
-        if ($result == mySociety::EmailUtil::EMAIL_SUCCESS) {
+        unless ($result) {
             print "  ...success\n" if $params->{verbose};
             $row->update();
             $token->insert();
diff --git a/perllib/FixMyStreet/EmailSend/ContactEmail.pm b/perllib/FixMyStreet/EmailSend/ContactEmail.pm
new file mode 100644
index 000000000..28bcc983b
--- /dev/null
+++ b/perllib/FixMyStreet/EmailSend/ContactEmail.pm
@@ -0,0 +1,9 @@
+package FixMyStreet::EmailSend::ContactEmail;
+use base Email::Send::SMTP;
+
+sub get_env_sender {
+    my $sender = FixMyStreet->config('CONTACT_EMAIL');
+    return $sender;
+}
+
+1;
diff --git a/perllib/FixMyStreet/EmailSend.pm b/perllib/FixMyStreet/EmailSend/DoNotReply.pm
index 8b6eed462..d1368f00f 100644
--- a/perllib/FixMyStreet/EmailSend.pm
+++ b/perllib/FixMyStreet/EmailSend/DoNotReply.pm
@@ -1,4 +1,4 @@
-package FixMyStreet::EmailSend;
+package FixMyStreet::EmailSend::DoNotReply;
 use base Email::Send::SMTP;
 
 sub get_env_sender {
diff --git a/perllib/FixMyStreet/SendReport.pm b/perllib/FixMyStreet/SendReport.pm
index 5087c7ead..40e76ef72 100644
--- a/perllib/FixMyStreet/SendReport.pm
+++ b/perllib/FixMyStreet/SendReport.pm
@@ -10,6 +10,7 @@ use Module::Pluggable
 has 'body_config' => ( is => 'rw', isa => 'HashRef', default => sub { {} } );
 has 'bodies' => ( is => 'rw', isa => 'ArrayRef', default => sub { [] } );
 has 'to' => ( is => 'rw', isa => 'ArrayRef', default => sub { [] } );
+has 'bcc' => ( is => 'rw', isa => 'ArrayRef', default => sub { [] } );
 has 'success' => ( is => 'rw', isa => 'Bool', default => 0 );
 has 'error' => ( is => 'rw', isa => 'Str', default => '' );
 has 'unconfirmed_counts' => ( 'is' => 'rw', isa => 'HashRef', default => sub { {} } );
@@ -44,6 +45,7 @@ sub reset {
     $self->bodies( [] );
     $self->body_config( {} );
     $self->to( [] );
+    $self->bcc( [] );
 }
 
 sub add_body {
diff --git a/perllib/FixMyStreet/SendReport/Email.pm b/perllib/FixMyStreet/SendReport/Email.pm
index 19c6405d2..797b41e91 100644
--- a/perllib/FixMyStreet/SendReport/Email.pm
+++ b/perllib/FixMyStreet/SendReport/Email.pm
@@ -4,11 +4,8 @@ use Moose;
 
 BEGIN { extends 'FixMyStreet::SendReport'; }
 
-use mySociety::EmailUtil;
-
 sub build_recipient_list {
     my ( $self, $row, $h ) = @_;
-    my %recips;
 
     my $all_confirmed = 1;
     foreach my $body ( @{ $self->bodies } ) {
@@ -49,12 +46,10 @@ sub build_recipient_list {
         }
         for my $email ( @emails ) {
             push @{ $self->to }, [ $email, $body_name ];
-            $recips{$email} = 1;
         }
     }
 
-    return () unless $all_confirmed;
-    return keys %recips;
+    return $all_confirmed && @{$self->to};
 }
 
 sub get_template {
@@ -76,34 +71,36 @@ sub send {
     my $self = shift;
     my ( $row, $h ) = @_;
 
-    my @recips = $self->build_recipient_list( $row, $h );
+    my $recips = $self->build_recipient_list( $row, $h );
 
     # on a staging server send emails to ourselves rather than the bodies
     if (mySociety::Config::get('STAGING_SITE') && !mySociety::Config::get('SEND_REPORTS_ON_STAGING') && !FixMyStreet->test_mode) {
-        @recips = ( $row->user->email );
+        $recips = 1;
+        @{$self->to} = [ $row->user->email, $self->to->[0][1] || $row->name ];
     }
 
-    unless ( @recips ) {
+    unless ($recips) {
         $self->error( 'No recipients' );
         return 1;
     }
 
     my ($verbose, $nomail) = CronFns::options();
     my $cobrand = FixMyStreet::Cobrand->get_class_for_moniker($row->cobrand)->new();
+    my $params = {
+        _template_ => $self->get_template( $row ),
+        _parameters_ => $h,
+        To => $self->to,
+        From => $self->send_from( $row ),
+    };
+    $params->{Bcc} = $self->bcc if @{$self->bcc};
     my $result = FixMyStreet::App->send_email_cron(
-        {
-            _template_ => $self->get_template( $row ),
-            _parameters_ => $h,
-            To => $self->to,
-            From => $self->send_from( $row ),
-        },
+        $params,
         mySociety::Config::get('CONTACT_EMAIL'),
-        \@recips,
         $nomail,
         $cobrand
     );
 
-    if ( $result == mySociety::EmailUtil::EMAIL_SUCCESS ) {
+    unless ($result) {
         $self->success(1);
     } else {
         $self->error( 'Failed to send email' );
@@ -143,4 +140,5 @@ sub _get_district_for_contact {
     ($district) = keys %$district;
     return $district;
 }
+
 1;
diff --git a/perllib/FixMyStreet/SendReport/EmptyHomes.pm b/perllib/FixMyStreet/SendReport/EmptyHomes.pm
index 4bae6af46..ce69aaac3 100644
--- a/perllib/FixMyStreet/SendReport/EmptyHomes.pm
+++ b/perllib/FixMyStreet/SendReport/EmptyHomes.pm
@@ -9,7 +9,6 @@ BEGIN { extends 'FixMyStreet::SendReport::Email'; }
 
 sub build_recipient_list {
     my ( $self, $row, $h ) = @_;
-    my %recips;
 
     my $all_confirmed = 1;
     foreach my $body ( @{ $self->bodies } ) {
@@ -31,24 +30,22 @@ sub build_recipient_list {
         }
 
         push @{ $self->to }, [ $body_email, $body->name ];
-        $recips{$body_email} = 1;
 
         my $area_info = mySociety::MaPit::call('area', $body->body_areas->first->area_id);
         my $country = $area_info->{country};
         if ($country eq 'W') {
-            $recips{ 'wales@' . mySociety::Config::get('EMAIL_DOMAIN') } = 1;
+            push @{$self->bcc}, 'wales@' . mySociety::Config::get('EMAIL_DOMAIN');
         } elsif ($country eq 'S') {
-            $recips{ 'scotland@' . mySociety::Config::get('EMAIL_DOMAIN') } = 1;
+            push @{$self->bcc}, 'scotland@' . mySociety::Config::get('EMAIL_DOMAIN');
         } else {
-            $recips{ 'eha@' . mySociety::Config::get('EMAIL_DOMAIN') } = 1;
+            push @{$self->bcc}, 'eha@' . mySociety::Config::get('EMAIL_DOMAIN');
         }
     }
 
     # Set address email parameter from added data
     $h->{address} = $row->extra->{address};
 
-    return () unless $all_confirmed;
-    return keys %recips;
+    return $all_confirmed && @{$self->to};
 }
 
 sub get_template {
diff --git a/perllib/FixMyStreet/SendReport/NI.pm b/perllib/FixMyStreet/SendReport/NI.pm
index e0ea24f9c..c60643566 100644
--- a/perllib/FixMyStreet/SendReport/NI.pm
+++ b/perllib/FixMyStreet/SendReport/NI.pm
@@ -6,7 +6,6 @@ BEGIN { extends 'FixMyStreet::SendReport::Email'; }
 
 sub build_recipient_list {
     my ( $self, $row, $h ) = @_;
-    my %recips;
 
     my $all_confirmed = 1;
     foreach my $body ( @{ $self->bodies } ) {
@@ -30,11 +29,9 @@ sub build_recipient_list {
             $row->external_body( 'Roads Service' );
         }
         push @{ $self->to }, [ $email, $name ];
-        $recips{$email} = 1;
     }
 
-    return () unless $all_confirmed;
-    return keys %recips;
+    return $all_confirmed && @{$self->to};
 }
 
 1;
diff --git a/perllib/FixMyStreet/SendReport/Zurich.pm b/perllib/FixMyStreet/SendReport/Zurich.pm
index d46561e9e..40417b41e 100644
--- a/perllib/FixMyStreet/SendReport/Zurich.pm
+++ b/perllib/FixMyStreet/SendReport/Zurich.pm
@@ -26,7 +26,7 @@ sub build_recipient_list {
     }
 
     push @{ $self->to }, [ $body_email, $body->name ];
-    return $body_email;
+    return 1;
 }
 
 sub get_template {