Decode template files read in with read_file, to prevent double encoding (fixes #159). Also fix Welsh I18N test.

7 files changed, 52 insertions, 15 deletions

diff --git a/bin/send-reports b/bin/send-reports
index 1af3ba1ea..9ab6f8274 100755
--- a/bin/send-reports
+++ b/bin/send-reports
@@ -12,7 +12,6 @@ require 5.8.0;
 use Digest::MD5;
 use Encode;
 use Error qw(:try);
-use File::Slurp;
 use JSON;
 use LWP::UserAgent;
 use LWP::Simple;
@@ -117,7 +116,7 @@ while (my $row = $unsent->next) {
         push @to, [ $council_email, $name ];
         @recips = ($council_email);
         $send_email = 1;
-        $template = File::Slurp::read_file("$FindBin::Bin/../templates/email/emptyhomes/" . $row->lang . "/submit.txt");
+        $template = Utils::read_file("$FindBin::Bin/../templates/email/emptyhomes/" . $row->lang . "/submit.txt");
 
     } else {
 
@@ -166,7 +165,7 @@ while (my $row = $unsent->next) {
         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 = File::Slurp::read_file( $template_path );
+        $template = Utils::read_file( $template_path );
 
         if ($h{category} eq _('Other')) {
             $h{category_footer} = _('this type of local problem');
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index ffbb5a161..1e6a3a088 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -666,7 +666,7 @@ sub process_report : Private {
     $report->detail( $detail );
 
     # set these straight from the params
-    $report->category( _ $params{category} );
+    $report->category( _ $params{category} ) if $params{category};
 
     my $areas = $c->stash->{all_areas};
     $report->areas( ',' . join( ',', sort keys %$areas ) . ',' );
diff --git a/perllib/FixMyStreet/DB/ResultSet/AlertType.pm b/perllib/FixMyStreet/DB/ResultSet/AlertType.pm
index bf085e32a..c1a5d65c9 100644
--- a/perllib/FixMyStreet/DB/ResultSet/AlertType.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/AlertType.pm
@@ -4,8 +4,6 @@ use base 'DBIx::Class::ResultSet';
 use strict;
 use warnings;
 
-use File::Slurp;
-
 use mySociety::DBHandle qw(dbh);
 use mySociety::EmailUtil;
 use mySociety::Gaze;
@@ -189,7 +187,7 @@ sub _send_aggregated_alert_email(%) {
         unless -e $template;
     $template = FixMyStreet->path_to( "templates", "email", "default", "$data{template}.txt" )->stringify
         unless -e $template;
-    $template = File::Slurp::read_file($template);
+    $template = Utils::read_file($template);
 
     my $sender = $cobrand->contact_email;
     (my $from = $sender) =~ s/team/fms-DO-NOT-REPLY/; # XXX
diff --git a/perllib/FixMyStreet/DB/ResultSet/Questionnaire.pm b/perllib/FixMyStreet/DB/ResultSet/Questionnaire.pm
index 665e0e3e0..0cf01b6d1 100644
--- a/perllib/FixMyStreet/DB/ResultSet/Questionnaire.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/Questionnaire.pm
@@ -3,7 +3,7 @@ use base 'DBIx::Class::ResultSet';
 
 use strict;
 use warnings;
-use File::Slurp;
+use Encode;
 use Utils;
 use mySociety::EmailUtil;
 
@@ -57,12 +57,12 @@ sub send_questionnaires_period {
         my $template;
         if ($params->{site} eq 'emptyhomes') {
             ($template = $period) =~ s/ //;
-            $template = File::Slurp::read_file( FixMyStreet->path_to( "templates/email/emptyhomes/" . $row->lang . "/questionnaire-$template.txt" )->stringify );
+            $template = Utils::read_file( FixMyStreet->path_to( "templates/email/emptyhomes/" . $row->lang . "/questionnaire-$template.txt" )->stringify );
         } else {
             $template = FixMyStreet->path_to( "templates", "email", $cobrand->moniker, "questionnaire.txt" )->stringify;
             $template = FixMyStreet->path_to( "templates", "email", "default", "questionnaire.txt" )->stringify
                 unless -e $template;
-            $template = File::Slurp::read_file( $template );
+            $template = Utils::read_file( $template );
         }
 
         my %h = map { $_ => $row->$_ } qw/name title detail category/;
diff --git a/perllib/Utils.pm b/perllib/Utils.pm
index c9afff186..954561a08 100644
--- a/perllib/Utils.pm
+++ b/perllib/Utils.pm
@@ -13,6 +13,7 @@ package Utils;
 
 use strict;
 use Encode;
+use File::Slurp qw();
 use POSIX qw(strftime);
 use mySociety::DBHandle qw(dbh);
 use mySociety::GeoUtil;
@@ -262,4 +263,17 @@ sub _part {
     }
 }
 
+=head2 read_file
+
+Reads in a UTF-8 encoded file using File::Slurp and decodes it from UTF-8.
+This appears simplest, rather than getting confused with binmodes and so on.
+
+=cut
+sub read_file {
+    my $filename = shift;
+    my $data = File::Slurp::read_file( $filename );
+    $data = Encode::decode( 'utf8', $data );
+    return $data;
+}
+
 1;
diff --git a/t/app/controller/questionnaire.t b/t/app/controller/questionnaire.t
index a013569ee..0e2a71184 100644
--- a/t/app/controller/questionnaire.t
+++ b/t/app/controller/questionnaire.t
@@ -28,7 +28,7 @@ my $report = FixMyStreet::App->model('DB::Problem')->find_or_create(
         areas              => ',11808,135007,14419,134935,2651,20728,',
         category           => 'Street lighting',
         title              => 'Testing',
-        detail             => 'Testing Detail',
+        detail             => "Testing \x{2013} Detail",
         used_map           => 1,
         name               => $user->name,
         anonymous          => 0,
@@ -38,7 +38,7 @@ my $report = FixMyStreet::App->model('DB::Problem')->find_or_create(
         whensent           => $sent_time,
         lang               => 'en-gb',
         service            => '',
-        cobrand            => 'default',
+        cobrand            => '',
         cobrand_data       => '',
         send_questionnaire => 1,
         latitude           => '55.951963',
@@ -56,6 +56,10 @@ FixMyStreet::App->model('DB::Questionnaire')->send_questionnaires( {
 my $email = $mech->get_email;
 ok $email, "got an email";
 like $email->body, qr/fill in our short questionnaire/i, "got questionnaire email";
+
+like $email->body, qr/Testing =96 Detail/, 'email contains encoded character';
+is $email->header('Content-Type'), 'text/plain; charset="windows-1252"', 'in the right character set';
+
 my ($token) = $email->body =~ m{http://.*?/Q/(\S+)};
 ok $token, "extracted questionnaire token '$token'";
 $mech->clear_emails_ok;
@@ -330,6 +334,8 @@ FixMyStreet::App->model('DB::Questionnaire')->send_questionnaires( {
 } );
 $email = $mech->get_email;
 ok $email, "got an email";
+$mech->clear_emails_ok;
+
 like $email->body, qr/fill in this short questionnaire/i, "got questionnaire email";
 ($token) = $email->body =~ m{http://.*?/Q/(\S+)};
 ok $token, "extracted questionnaire token '$token'";
@@ -346,7 +352,7 @@ my $questionnaire2 = FixMyStreet::App->model('DB::Questionnaire')->find_or_creat
         ever_reported => 1,
     }
 );
-ok $questionnaire, 'added another questionnaire';
+ok $questionnaire2, 'added another questionnaire';
 ok $mech->host("fixmystreet.com"), 'change host to fixmystreet';
 $mech->get_ok("/Q/" . $token);
 $mech->title_like( qr/Questionnaire/ );
@@ -358,5 +364,25 @@ ok $mech->host("reportemptyhomes.com"), 'change host to reportemptyhomes';
 $mech->get_ok("/Q/" . $token);
 $mech->content_contains( 'made a lot of progress' );
 
+$token = FixMyStreet::App->model("DB::Token")->find( { scope => 'questionnaire', token => $token } );
+ok $token, 'found token for questionnaire';
+$questionnaire = FixMyStreet::App->model('DB::Questionnaire')->find( { id => $token->data } );
+ok $questionnaire, 'found questionnaire';
+
+# I18N Unicode extra testing using FiksGataMi
+$report->send_questionnaire( 1 );
+$report->cobrand( 'fiksgatami' );
+$report->update;
+$questionnaire->delete;
+$questionnaire2->delete;
+FixMyStreet::App->model('DB::Questionnaire')->send_questionnaires( { site => 'fixmystreet' } ); # It's either fixmystreet or emptyhomes
+$email = $mech->get_email;
+ok $email, "got an email";
+$mech->clear_emails_ok;
+
+like $email->body, qr/Testing =96 Detail/, 'email contains encoded character from user';
+like $email->body, qr/sak p=E5 FiksGataMi/, 'email contains encoded character from template';
+is $email->header('Content-Type'), 'text/plain; charset="windows-1252"', 'email is in right encoding';
+
 $mech->delete_user('test@example.com');
 done_testing();
diff --git a/t/i18n.t b/t/i18n.t
index 2279ca6a8..4179dd8d0 100644
--- a/t/i18n.t
+++ b/t/i18n.t
@@ -19,8 +19,8 @@ die "You need to run 'commonlib/bin/gettext-makemo --quiet FixMyStreet' "
     'locale/cy_GB.UTF-8/LC_MESSAGES/FixMyStreet-EmptyHomes.mo');
 
 # Example strings
-my $english = "Sorry! Something's gone wrong.";
-my $welsh   = "Ymddiheuriadau! Mae rhywbeth wedi mynd o'i le.";
+my $english = "Please enter a valid email";
+my $welsh   = "Cofnodwch gyfeiriad e-bost dilys";
 
 # set english as the language
 mySociety::Locale::negotiate_language(    #