Use only one templating system for emails.

Historically, emails sent offline (alerts, questionnaires, etc) used a
different templating system from those sent by the website (e.g. login
emails), though the newer system was also being used for the site name
and signature of offline emails.

9 files changed, 79 insertions, 103 deletions

diff --git a/perllib/FixMyStreet.pm b/perllib/FixMyStreet.pm
index 1a9c8ff60..14f3f3607 100644
--- a/perllib/FixMyStreet.pm
+++ b/perllib/FixMyStreet.pm
@@ -190,23 +190,6 @@ sub configure_mysociety_dbhandle {
 
 }
 
-=head2 get_email_template
-
-=cut
-
-sub get_email_template {
-    # TODO further refactor this by just using Template path
-    my ($class, $cobrand, $lang, $template) = @_;
-
-    my $template_path = FixMyStreet->path_to( "templates", "email", $cobrand, $lang, $template )->stringify;
-    $template_path = FixMyStreet->path_to( "templates", "email", $cobrand, $template )->stringify
-        unless -e $template_path;
-    $template_path = FixMyStreet->path_to( "templates", "email", "default", $template )->stringify
-        unless -e $template_path;
-    $template = Utils::read_file( $template_path );
-    return $template;
-}
-
 my $tz;
 my $tz_f;
 
diff --git a/perllib/FixMyStreet/App.pm b/perllib/FixMyStreet/App.pm
index 79ca7f9ee..1a651d282 100644
--- a/perllib/FixMyStreet/App.pm
+++ b/perllib/FixMyStreet/App.pm
@@ -320,8 +320,7 @@ sub send_email {
 
     my $email = mySociety::Locale::in_gb_locale { FixMyStreet::Email::construct_email(
         {
-            _template_ => $c->view('Email')->render( $c, $template, $vars ),
-            _parameters_ => {},
+            _body_ => $c->view('Email')->render( $c, $template, $vars ),
             _attachments_ => $extra_stash_values->{attachments},
             From => $vars->{from},
             To => $vars->{to},
diff --git a/perllib/FixMyStreet/Cobrand/Zurich.pm b/perllib/FixMyStreet/Cobrand/Zurich.pm
index d31b1c84e..987f0bb67 100644
--- a/perllib/FixMyStreet/Cobrand/Zurich.pm
+++ b/perllib/FixMyStreet/Cobrand/Zurich.pm
@@ -587,14 +587,6 @@ sub admin_report_edit {
         #    2) setting $problem->whensent(undef) may make it eligible for generating an email
         #   to the body (internal or external).  See DBRS::Problem->send_reports for Zurich-
         #   specific categories which are eligible for this.
-        #
-        #   It looks like both of these will do:
-        #       a) TT processing of [% ... %] directives, in FMS::App->send_email(_cron)
-        #       b) pseudo-PHP substitution of <?=$values['name']?> which is done as
-        #       naive substitution
-        #       commonlib mySociety::Email
-        #
-        #   So it makes sense to add new parameters as the more powerful TT (a).
 
         my $redirect = 0;
         my $new_cat = $c->get_param('category') || '';
diff --git a/perllib/FixMyStreet/Email.pm b/perllib/FixMyStreet/Email.pm
index 49f4632a8..d4bfee14e 100644
--- a/perllib/FixMyStreet/Email.pm
+++ b/perllib/FixMyStreet/Email.pm
@@ -1,3 +1,9 @@
+package FixMyStreet::Email::Error;
+
+use Error qw(:try);
+
+@FixMyStreet::Email::Error::ISA = qw(Error::Simple);
+
 package FixMyStreet::Email;
 
 use Email::MIME;
@@ -5,7 +11,7 @@ use Encode;
 use POSIX qw();
 use Template;
 use Digest::HMAC_SHA1 qw(hmac_sha1_hex);
-use mySociety::Email;
+use Text::Wrap;
 use mySociety::Locale;
 use mySociety::Random qw(random_bytes);
 use Utils::Email;
@@ -64,50 +70,42 @@ sub is_abuser {
     return $schema->resultset('Abuse')->search( { email => [ $email, $domain ] } )->first;
 }
 
+sub _render_template {
+    my ($tt, $template, $vars, %options) = @_;
+    my $var;
+    $tt->process($template, $vars, \$var);
+    return $var;
+}
+
 sub send_cron {
-    my ( $schema, $params, $env_from, $nomail, $cobrand, $lang_code ) = @_;
+    my ( $schema, $template, $vars, $hdrs, $env_from, $nomail, $cobrand, $lang_code ) = @_;
 
     my $sender = FixMyStreet->config('DO_NOT_REPLY_EMAIL');
     $env_from ||= $sender;
-    if (!$params->{From}) {
+    if (!$hdrs->{From}) {
         my $sender_name = $cobrand->contact_name;
-        $params->{From} = [ $sender, _($sender_name) ];
+        $hdrs->{From} = [ $sender, _($sender_name) ];
     }
 
-    return 1 if is_abuser($schema, $params->{To});
+    return 1 if is_abuser($schema, $hdrs->{To});
 
-    $params->{'Message-ID'} = sprintf('<fms-cron-%s-%s@%s>', time(),
+    $hdrs->{'Message-ID'} = sprintf('<fms-cron-%s-%s@%s>', time(),
         unpack('h*', random_bytes(5, 1)), FixMyStreet->config('EMAIL_DOMAIN')
     );
 
-    # This is all to set the path for the templates processor so we can override
-    # signature and site names in emails using templates in the old style emails.
-    # It's a bit involved as not everywhere we use it knows about the cobrand so
-    # we can't assume there will be one.
-    my $include_path = FixMyStreet->path_to( 'templates', 'email', 'default' )->stringify;
-    if ( $cobrand ) {
-        $include_path =
-            FixMyStreet->path_to( 'templates', 'email', $cobrand->moniker )->stringify . ':'
-            . $include_path;
-        if ( $lang_code ) {
-            $include_path =
-                FixMyStreet->path_to( 'templates', 'email', $cobrand->moniker, $lang_code )->stringify . ':'
-                . $include_path;
-        }
-    }
     my $tt = Template->new({
-        INCLUDE_PATH => $include_path
+        ENCODING => 'utf8',
+        INCLUDE_PATH => [
+            FixMyStreet->path_to( 'templates', 'email', $cobrand->moniker, $lang_code )->stringify,
+            FixMyStreet->path_to( 'templates', 'email', $cobrand->moniker )->stringify,
+            FixMyStreet->path_to( 'templates', 'email', 'default' )->stringify,
+        ],
     });
-    my ($sig, $site_name);
-    $tt->process( 'signature.txt', $params, \$sig );
-    $sig = Encode::decode('utf8', $sig);
-    $params->{_parameters_}->{signature} = $sig;
+    $vars->{signature} = _render_template($tt, 'signature.txt', $vars);
+    $vars->{site_name} = Utils::trim_text(_render_template($tt, 'site-name.txt', $vars));
+    $hdrs->{_body_} = _render_template($tt, $template, $vars);
 
-    $tt->process( 'site-name.txt', $params, \$site_name );
-    $site_name = Utils::trim_text(Encode::decode('utf8', $site_name));
-    $params->{_parameters_}->{site_name} = $site_name;
-
-    my $email = mySociety::Locale::in_gb_locale { construct_email($params) };
+    my $email = mySociety::Locale::in_gb_locale { construct_email($hdrs) };
 
     if ($nomail) {
         print $email->as_string;
@@ -125,16 +123,12 @@ containing elements as given below. Returns an Email::MIME email.
 
 =over 4
 
-=item _template_, _parameters_
+=item _body_
 
-Templated body text and an associative array of template parameters. _template
-contains optional substititutions <?=$values['name']?>, each of which is
-replaced by the value of the corresponding named value in _parameters_. It is
-an error to use a substitution when the corresponding parameter is not present
-or undefined. The first line of the template will be interpreted as contents of
+Body text. The first line of the template will be interpreted as contents of
 the Subject: header of the mail if it begins with the literal string 'Subject:
-' followed by a blank line. The templated text will be word-wrapped to produce
-lines of appropriate length.
+' followed by a blank line. The text will be word-wrapped to produce lines of
+appropriate length.
 
 =item _attachments_
 
@@ -175,21 +169,42 @@ templated body, From or Subject (perhaps from the template).
 sub construct_email ($) {
     my $p = shift;
 
-    throw mySociety::Email::Error("Must specify both '_template_' and '_parameters_'")
-        if !exists($p->{_template_}) || !exists($p->{_parameters_});
-    throw mySociety::Email::Error("Template parameters '_parameters_' must be an associative array")
-        if (ref($p->{_parameters_}) ne 'HASH');
+    throw FixMyStreet::Email::Error("Must specify '_body_'") if !exists($p->{_body_});
+
+    my $body = $p->{_body_};
+    my $subject;
+    if ($body =~ m#^Subject: ([^\n]*)\n\n#s) {
+        $subject = $1;
+        $body =~ s#^Subject: ([^\n]*)\n\n##s;
+    }
+
+    $body =~ s/\r\n/\n/gs;
+    $body =~ s/^\s+$//mg; # Note this also reduces any gap between paragraphs of >1 blank line to 1
+    $body =~ s/\s+$//;
+
+    # Merge paragraphs into their own line.  Two blank lines separate a
+    # paragraph. End a line with two spaces to force a linebreak.
+
+    # regex means, "replace any line ending that is neither preceded (?<!\n)
+    # nor followed (?!\n) by a blank line with a single space".
+    $body =~ s#(?<!\n)(?<!  )\n(?!\n)# #gs;
+
+    # Wrap text to 72-column lines.
+    local($Text::Wrap::columns) = 69;
+    local($Text::Wrap::huge) = 'overflow';
+    local($Text::Wrap::unexpand) = 0;
+    $body = Text::Wrap::wrap('', '', $body);
+    $body =~ s/^\s+$//mg; # Do it again because of wordwrapping indented lines
 
-    (my $subject, $body) = mySociety::Email::do_template_substitution($p->{_template_}, $p->{_parameters_}, '');
     $p->{Subject} = $subject if defined($subject);
 
     if (!exists($p->{Subject})) {
         # XXX Try to find out what's causing this very occasionally
         (my $error = $body) =~ s/\n/ | /g;
         $error = "missing field 'Subject' in MESSAGE - $error";
-        throw mySociety::Email::Error($error);
+        throw FixMyStreet::Email::Error($error);
     }
-    throw mySociety::Email::Error("missing field 'From' in MESSAGE") unless exists($p->{From});
+    throw FixMyStreet::Email::Error("missing field 'From' in MESSAGE") unless exists($p->{From});
 
     # Construct email headers
     my %hdr;
@@ -203,7 +218,7 @@ sub construct_email ($) {
             # Array of addresses or [address, name] pairs.
             $hdr{$h} = join(', ', map { mailbox($_, $h) } @{$p->{$h}});
         } else {
-            throw mySociety::Email::Error("Field '$h' in MESSAGE should be single value or an array");
+            throw FixMyStreet::Email::Error("Field '$h' in MESSAGE should be single value or an array");
         }
     }
 
@@ -251,7 +266,7 @@ sub mailbox {
     if (ref($e) eq '') {
         return $e;
     } elsif (ref($e) ne 'ARRAY' || @$e != 2) {
-        throw mySociety::Email::Error("'$header' field should be string or 2-element array");
+        throw FixMyStreet::Email::Error("'$header' field should be string or 2-element array");
     } else {
         return Email::Address->new($e->[1], $e->[0]);
     }
diff --git a/perllib/FixMyStreet/Script/Alerts.pm b/perllib/FixMyStreet/Script/Alerts.pm
index fc5fef953..062601044 100644
--- a/perllib/FixMyStreet/Script/Alerts.pm
+++ b/perllib/FixMyStreet/Script/Alerts.pm
@@ -248,8 +248,6 @@ sub _send_aggregated_alert_email(%) {
     } );
     $data{unsubscribe_url} = $cobrand->base_url( $data{cobrand_data} ) . '/A/' . $token->token;
 
-    my $template = FixMyStreet->get_email_template($cobrand->moniker, $data{lang}, "$data{template}.txt");
-
     my $sender = sprintf('<fms-%s@%s>',
         FixMyStreet::Email::generate_verp_token('alert', $data{alert_id}),
         FixMyStreet->config('EMAIL_DOMAIN')
@@ -257,9 +255,9 @@ sub _send_aggregated_alert_email(%) {
 
     my $result = FixMyStreet::Email::send_cron(
         $data{schema},
+        "$data{template}.txt",
+        \%data,
         {
-            _template_ => $template,
-            _parameters_ => \%data,
             To => $data{alert_email},
         },
         $sender,
diff --git a/perllib/FixMyStreet/Script/Questionnaires.pm b/perllib/FixMyStreet/Script/Questionnaires.pm
index f72f59077..c5bc6bfe0 100644
--- a/perllib/FixMyStreet/Script/Questionnaires.pm
+++ b/perllib/FixMyStreet/Script/Questionnaires.pm
@@ -52,8 +52,6 @@ sub send_questionnaires_period {
         # call checks if this is the host that sends mail for this cobrand.
         next unless $cobrand->email_host;
 
-        my $template = FixMyStreet->get_email_template($cobrand->moniker, $row->lang, 'questionnaire.txt');
-
         my %h = map { $_ => $row->$_ } qw/name title detail category/;
         $h{created} = Utils::prettify_duration( time() - $row->confirmed->epoch, 'week' );
 
@@ -78,14 +76,15 @@ sub send_questionnaires_period {
 
         my $result = FixMyStreet::Email::send_cron(
             $rs->result_source->schema,
+            'questionnaire.txt',
+            \%h,
             {
-                _template_ => $template,
-                _parameters_ => \%h,
                 To => [ [ $row->user->email, $row->name ] ],
             },
             undef,
             $params->{nomail},
-            $cobrand
+            $cobrand,
+            $row->lang,
         );
         unless ($result) {
             print "  ...success\n" if $params->{verbose};
diff --git a/perllib/FixMyStreet/Script/Reports.pm b/perllib/FixMyStreet/Script/Reports.pm
index 75111b852..278c58af1 100644
--- a/perllib/FixMyStreet/Script/Reports.pm
+++ b/perllib/FixMyStreet/Script/Reports.pm
@@ -289,19 +289,18 @@ sub _send_report_sent_email {
     my $nomail = shift;
     my $cobrand = shift;
 
-    my $template = FixMyStreet->get_email_template($row->cobrand, $row->lang, 'confirm_report_sent.txt');
-
     FixMyStreet::Email::send_cron(
         $row->result_source->schema,
+        'confirm_report_sent.txt',
+        $h,
         {
-            _template_ => $template,
-            _parameters_ => $h,
             To => $row->user->email,
             From => [ FixMyStreet->config('CONTACT_EMAIL'), $cobrand->contact_name ],
         },
         FixMyStreet->config('CONTACT_EMAIL'),
         $nomail,
-        $cobrand
+        $cobrand,
+        $row->lang,
     );
 }
 
diff --git a/perllib/FixMyStreet/SendReport/Email.pm b/perllib/FixMyStreet/SendReport/Email.pm
index 7e5c10469..8582ebb3b 100644
--- a/perllib/FixMyStreet/SendReport/Email.pm
+++ b/perllib/FixMyStreet/SendReport/Email.pm
@@ -52,15 +52,8 @@ sub build_recipient_list {
 
 sub get_template {
     my ( $self, $row ) = @_;
-
-    my $template = 'submit.txt';
-
-    if ($row->cobrand eq 'fixmystreet') {
-        $template = 'submit-oxfordshire.txt' if $row->bodies_str eq 2237;
-    }
-
-    $template = FixMyStreet->get_email_template($row->cobrand, $row->lang, $template);
-    return $template;
+    return 'submit-oxfordshire.txt' if $row->cobrand eq 'fixmystreet' && $row->bodies_str eq 2237;
+    return 'submit.txt';
 }
 
 sub send_from {
@@ -88,8 +81,6 @@ sub send {
     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 ),
     };
@@ -108,7 +99,9 @@ sub send {
         $params->{From} = [ $sender, $params->{From}[1] ];
     }
 
-    my $result = FixMyStreet::Email::send_cron($row->result_source->schema, $params, $sender, $nomail, $cobrand);
+    my $result = FixMyStreet::Email::send_cron($row->result_source->schema,
+        $self->get_template($row), $h,
+        $params, $sender, $nomail, $cobrand, $row->lang);
 
     unless ($result) {
         $self->success(1);
diff --git a/perllib/FixMyStreet/SendReport/Zurich.pm b/perllib/FixMyStreet/SendReport/Zurich.pm
index a8730bbe4..b38981d94 100644
--- a/perllib/FixMyStreet/SendReport/Zurich.pm
+++ b/perllib/FixMyStreet/SendReport/Zurich.pm
@@ -59,8 +59,6 @@ sub get_template {
         }
     }
 
-    my $template_path = FixMyStreet->path_to( "templates", "email", "zurich", $template )->stringify;
-    $template = Utils::read_file( $template_path );
     return $template;
 }