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.

4 files changed, 21 insertions, 29 deletions

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 {