Simplify SendReport code, moving some Email only bits to that module.

8 files changed, 45 insertions, 43 deletions

diff --git a/perllib/FixMyStreet/DB/ResultSet/Problem.pm b/perllib/FixMyStreet/DB/ResultSet/Problem.pm
index a1f0c196a..00646748d 100644
--- a/perllib/FixMyStreet/DB/ResultSet/Problem.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/Problem.pm
@@ -287,16 +287,14 @@ sub send_reports {
         }
 
         my %reporters = ();
-        my (@to, @recips, $template, $areas_info, $sender_count );
+        my ( $sender_count );
         if ($site eq 'emptyhomes') {
 
             my $council = $row->council;
-            $areas_info = mySociety::MaPit::call('areas', $council);
-            my $name = $areas_info->{$council}->{name};
+            my $areas_info = mySociety::MaPit::call('areas', $council);
             my $sender = "FixMyStreet::SendReport::EmptyHomes";
             $reporters{ $sender } = $sender->new() unless $reporters{$sender};
-            $reporters{ $sender }->add_council( $council, $name );
-            $template = Utils::read_file("$FindBin::Bin/../templates/email/emptyhomes/" . $row->lang . "/submit.txt");
+            $reporters{ $sender }->add_council( $council, $areas_info->{$council} );
 
         } else {
 
@@ -304,7 +302,7 @@ sub send_reports {
             my @all_councils = split /,|\|/, $row->council;
             my ($councils, $missing) = $row->council =~ /^([\d,]+)(?:\|([\d,]+))?/;
             my @councils = split(/,/, $councils);
-            $areas_info = mySociety::MaPit::call('areas', \@all_councils);
+            my $areas_info = mySociety::MaPit::call('areas', \@all_councils);
             my @dear;
 
             foreach my $council (@councils) {
@@ -324,17 +322,10 @@ sub send_reports {
                         $reporters{ $sender }->skipped;
                 } else {
                     push @dear, $name;
-                    $reporters{ $sender }->add_council( $council, $name );
+                    $reporters{ $sender }->add_council( $council, $areas_info->{$council} );
                 }
             }
 
-            $template = 'submit.txt';
-            $template = 'submit-brent.txt' if $row->council eq 2488 || $row->council eq 2237;
-            my $template_path = FixMyStreet->path_to( "templates", "email", $cobrand->moniker, $template )->stringify;
-            $template_path = FixMyStreet->path_to( "templates", "email", "default", $template )->stringify
-                unless -e $template_path;
-            $template = Utils::read_file( $template_path );
-
             if ($h{category} eq _('Other')) {
                 $h{category_footer} = _('this type of local problem');
                 $h{category_line} = '';
@@ -382,9 +373,7 @@ sub send_reports {
         my $result = -1;
 
         for my $sender ( keys %reporters ) {
-            $result *= $reporters{ $sender }->send(
-                $row, \%h, \@to, $template, \@recips, $nomail, $areas_info
-            );
+            $result *= $reporters{ $sender }->send( $row, \%h );
         }
 
         if ($result == mySociety::EmailUtil::EMAIL_SUCCESS) {
diff --git a/perllib/FixMyStreet/SendReport.pm b/perllib/FixMyStreet/SendReport.pm
index 915fe4a20..66a31ffcc 100644
--- a/perllib/FixMyStreet/SendReport.pm
+++ b/perllib/FixMyStreet/SendReport.pm
@@ -36,9 +36,9 @@ sub reset {
 sub add_council {
     my $self = shift;
     my $council = shift;
-    my $name = shift;
+    my $info = shift;
 
-    $self->councils->{ $council } = $name;
+    $self->councils->{ $council } = $info;
 }
 
 
diff --git a/perllib/FixMyStreet/SendReport/Barnet.pm b/perllib/FixMyStreet/SendReport/Barnet.pm
index ecbe82872..9a92686ec 100644
--- a/perllib/FixMyStreet/SendReport/Barnet.pm
+++ b/perllib/FixMyStreet/SendReport/Barnet.pm
@@ -19,15 +19,15 @@ use constant MAX_LINE_LENGTH       => 132;
 
 sub should_skip {
     my $self = shift;
-    my $problem = shift;
+    my $row = shift;
 
     my $council_name = 'Barnet';
     my $err_msg = "";
 
-    if ($problem->send_fail_count >= SEND_FAIL_RETRIES_CUTOFF) {
+    if ($row->send_fail_count >= SEND_FAIL_RETRIES_CUTOFF) {
         $council_name &&= " to $council_name";
-        $err_msg = "skipped: problem id=" . $problem->id . " send$council_name has failed " 
-                . $problem->send_fail_count . " times, cutoff is " . SEND_FAIL_RETRIES_CUTOFF;
+        $err_msg = "skipped: problem id=" . $row->id . " send$council_name has failed " 
+                . $row->send_fail_count . " times, cutoff is " . SEND_FAIL_RETRIES_CUTOFF;
 
         $self->skipped( $err_msg );
 
@@ -52,7 +52,7 @@ EOF
 
 
 sub send {
-    my ( $self, $problem, $h, $to, $template, $recips, $nomail ) = @_;
+    my ( $self, $row, $h ) = @_;
 
     my %h = %$h;
 
@@ -127,9 +127,9 @@ sub send {
         if ($result) {
             # currently not using this: get_EV_ORDER_GUID (maybe that's the customer number in the CRM)
             if (my $barnet_id = $result->get_EV_ORDER_NO()) {
-                $problem->external_id( $barnet_id );
-                $problem->external_body( 'Barnet Borough Council' ); # better to use $problem->body()?
-                $problem->send_method_used('barnet');
+                $row->external_id( $barnet_id );
+                $row->external_body( 'Barnet Borough Council' ); # better to use $row->body()?
+                $row->send_method_used('barnet');
                 $return = 0;
             } else {
                 my @returned_items = split  /<item[^>]*>/, $result->get_ET_RETURN;
diff --git a/perllib/FixMyStreet/SendReport/EastHants.pm b/perllib/FixMyStreet/SendReport/EastHants.pm
index 681a9d4c4..beab9d55c 100644
--- a/perllib/FixMyStreet/SendReport/EastHants.pm
+++ b/perllib/FixMyStreet/SendReport/EastHants.pm
@@ -32,7 +32,7 @@ EOF
 sub send {
     return if mySociety::Config::get('STAGING_SITE');
 
-    my ( $self, $row, $h, $to, $template, $recips, $nomail ) = @_;
+    my ( $self, $row, $h ) = @_;
 
     # FIXME: should not recreate this each time
     my $eh_service;
diff --git a/perllib/FixMyStreet/SendReport/Email.pm b/perllib/FixMyStreet/SendReport/Email.pm
index 239bee715..dec204b1c 100644
--- a/perllib/FixMyStreet/SendReport/Email.pm
+++ b/perllib/FixMyStreet/SendReport/Email.pm
@@ -7,8 +7,7 @@ BEGIN { extends 'FixMyStreet::SendReport'; }
 use mySociety::EmailUtil;
 
 sub build_recipient_list {
-    my $self = shift;
-    my $row = shift;
+    my ( $self, $row, $h ) = @_;
     my %recips;
 
     my $all_confirmed = 1;
@@ -33,7 +32,7 @@ sub build_recipient_list {
             #$note{$council_email}{$row->category} = $note;
         }
 
-        push @{ $self->to }, [ $council_email, $self->councils->{ $council } ];
+        push @{ $self->to }, [ $council_email, $self->councils->{ $council }->{name} ];
         $recips{$council_email} = 1;
     }
 
@@ -41,13 +40,23 @@ sub build_recipient_list {
     return keys %recips;
 }
 
+sub get_template {
+    my ( $self, $row ) = @_;
+
+    my $template = 'submit.txt';
+    $template = 'submit-brent.txt' if $row->council eq 2488 || $row->council eq 2237;
+    my $template_path = FixMyStreet->path_to( "templates", "email", $row->cobrand, $template )->stringify;
+    $template_path = FixMyStreet->path_to( "templates", "email", "default", $template )->stringify
+        unless -e $template_path;
+    $template = Utils::read_file( $template_path );
+    return $template;
+}
+
 sub send {
     my $self = shift;
-    my ( $row, $h, $to, $template, $recips, $nomail, $areas_info ) = @_;
-
-    my @recips;
+    my ( $row, $h ) = @_;
 
-    @recips = $self->build_recipient_list( $row, $areas_info );
+    my @recips = $self->build_recipient_list( $row, $h );
 
     # on a staging server send emails to ourselves rather than the councils
     if (mySociety::Config::get('STAGING_SITE')) {
@@ -56,9 +65,10 @@ sub send {
 
     return unless @recips;
 
+    my ($verbose, $nomail) = CronFns::options();
     my $result = FixMyStreet::App->send_email_cron(
         {
-            _template_ => $template,
+            _template_ => $self->get_template( $row ),
             _parameters_ => $h,
             To => $self->to,
             From => [ $row->user->email, $row->name ],
diff --git a/perllib/FixMyStreet/SendReport/EmptyHomes.pm b/perllib/FixMyStreet/SendReport/EmptyHomes.pm
index 6c4a253d3..e1b914523 100644
--- a/perllib/FixMyStreet/SendReport/EmptyHomes.pm
+++ b/perllib/FixMyStreet/SendReport/EmptyHomes.pm
@@ -6,9 +6,7 @@ use namespace::autoclean;
 BEGIN { extends 'FixMyStreet::SendReport::Email'; }
 
 sub build_recipient_list {
-    my $self = shift;
-    my $row = shift;
-    my $areas_info = shift;
+    my ( $self, $row, $h ) = @_;
     my %recips;
 
     my $all_confirmed = 1;
@@ -30,10 +28,10 @@ sub build_recipient_list {
             #$note{$council_email}{$row->category} = $note;
         }
 
-        push @{ $self->to }, [ $council_email, $self->councils->{ $council } ];
+        push @{ $self->to }, [ $council_email, $self->councils->{ $council }->{name} ];
         $recips{$council_email} = 1;
 
-        my $country = $areas_info->{$council}->{country};
+        my $country = $self->councils->{$council}->{country};
         if ($country eq 'W') {
             $recips{ 'shelter@' . mySociety::Config::get('EMAIL_DOMAIN') } = 1;
         } else {
@@ -45,4 +43,9 @@ sub build_recipient_list {
     return keys %recips;
 }
 
+sub get_template {
+    my ( $self, $row ) = @_;
+    return Utils::read_file( FixMyStreet->path_to( "templates", "email", "emptyhomes", $row->lang, "submit.txt" )->stringify );
+}
+
 1;
diff --git a/perllib/FixMyStreet/SendReport/London.pm b/perllib/FixMyStreet/SendReport/London.pm
index 58ecb2375..6e7951922 100644
--- a/perllib/FixMyStreet/SendReport/London.pm
+++ b/perllib/FixMyStreet/SendReport/London.pm
@@ -33,7 +33,7 @@ EOF
 
 sub send {
     return if mySociety::Config::get('STAGING_SITE');
-    my ( $self, $row, $h, $to, $template, $recips, $nomail ) = @_;
+    my ( $self, $row, $h ) = @_;
 
     $h->{message} = construct_message( %$h );
     my $phone = $h->{phone};
diff --git a/perllib/FixMyStreet/SendReport/Open311.pm b/perllib/FixMyStreet/SendReport/Open311.pm
index d00965195..6584bf8e2 100644
--- a/perllib/FixMyStreet/SendReport/Open311.pm
+++ b/perllib/FixMyStreet/SendReport/Open311.pm
@@ -23,7 +23,7 @@ sub should_skip {
 
 sub send {
     my $self = shift;
-    my ( $row, $h, $to, $template, $recips, $nomail ) = @_;
+    my ( $row, $h ) = @_;
 
     my $result = -1;