Merge branch 'bexley-stop-ooo'

mysociety · Nov 20, 2020 · 86b0323 · 86b0323
2 parents 96192a0 + 79d9cc2
commit 86b0323
Show file tree

Hide file tree

Showing 10 changed files with 500 additions and 487 deletions.
diff --git a/perllib/FixMyStreet/Cobrand/Bexley.pm b/perllib/FixMyStreet/Cobrand/Bexley.pm
@@ -194,7 +194,11 @@ sub open311_post_send {
     }
 
     return unless @to;
-    my $sender = FixMyStreet::SendReport::Email->new( to => \@to );
+    my $sender = FixMyStreet::SendReport::Email->new(
+        use_verp => 0,
+        use_replyto => 1,
+        to => \@to,
+    );
 
     $self->open311_config($row, $h, {}, $contact); # Populate NSGRef again if needed
 

diff --git a/perllib/FixMyStreet/SendReport.pm b/perllib/FixMyStreet/SendReport.pm
@@ -11,13 +11,10 @@ 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_data' => ( 'is' => 'rw', isa => HashRef, default => sub { {} } );
 
-
 sub get_senders {
     my $self = shift;
 
@@ -26,15 +23,6 @@ sub get_senders {
     return \%senders;
 }
 
-sub reset {
-    my $self = shift;
-
-    $self->bodies( [] );
-    $self->body_config( {} );
-    $self->to( [] );
-    $self->bcc( [] );
-}
-
 sub add_body {
     my $self = shift;
     my $body = shift;

diff --git a/perllib/FixMyStreet/SendReport/Email.pm b/perllib/FixMyStreet/SendReport/Email.pm
@@ -1,11 +1,18 @@
 package FixMyStreet::SendReport::Email;
 
 use Moo;
+use MooX::Types::MooseLike::Base qw(:all);
 use FixMyStreet::Email;
 use Utils::Email;
 
 BEGIN { extends 'FixMyStreet::SendReport'; }
 
+has to => ( is => 'ro', isa => ArrayRef, default => sub { [] } );
+has bcc => ( is => 'ro', isa => ArrayRef, default => sub { [] } );
+
+has use_verp => ( is => 'ro', isa => Int, default => 1 );
+has use_replyto => ( is => 'ro', isa => Int, default => 0 );
+
 sub build_recipient_list {
     my ( $self, $row, $h ) = @_;
 
@@ -54,7 +61,7 @@ sub envelope_sender {
     my ($self, $row) = @_;
 
     my $cobrand = $row->get_cobrand_logged;
-    if ($row->user->email && $row->user->email_verified) {
+    if ($self->use_verp && $row->user->email && $row->user->email_verified) {
         return FixMyStreet::Email::unique_verp_id([ 'report', $row->id ], $cobrand->call_hook('verp_email_domain'));
     }
     return $cobrand->do_not_reply_email;
@@ -98,6 +105,7 @@ sub send {
     }
 
     if (FixMyStreet::Email::test_dmarc($params->{From}[0])
+      || $self->use_replyto
       || Utils::Email::same_domain($params->{From}, $params->{To})) {
         $params->{'Reply-To'} = [ $params->{From} ];
         $params->{From} = [ $sender, $params->{From}[1] ];

diff --git a/t/app/sendreport/open311.t b/t/app/sendreport/open311.t
diff --git a/t/cobrand/bexley.t b/t/cobrand/bexley.t
@@ -129,6 +129,7 @@ FixMyStreet::override_config {
             if (my $t = $test->{email}) {
                 my $email = $mech->get_email;
                 $t = join('@[^@]*', @$t);
+                is $email->header('From'), '"Test User" <do-not-reply@example.org>';
                 like $email->header('To'), qr/^[^@]*$t@[^@]*$/;
                 if ($test->{code} =~ /Confirm/) {
                     like $mech->get_text_body_from_email($email), qr/Site code: Road ID/;

diff --git a/t/app/sendreport/blackhole.t → t/sendreport/blackhole.t b/t/app/sendreport/blackhole.t → t/sendreport/blackhole.t