Move most of questionnaire cron to ResultSet so it can be called and tested from test.

5 files changed, 192 insertions, 155 deletions

diff --git a/bin/send-questionnaires b/bin/send-questionnaires
index 0a42afe8b..d6e269e32 100755
--- a/bin/send-questionnaires
+++ b/bin/send-questionnaires
@@ -9,126 +9,17 @@
 use strict;
 require 5.8.0;
 
-use File::Slurp;
 use CGI; # XXX Awkward kludge
 use Encode;
 use CronFns;
 
 use FixMyStreet::App;
-
-use Page;
 use mySociety::Config;
-use mySociety::Email;
-use mySociety::Locale;
-use mySociety::EmailUtil;
-use mySociety::Random qw(random_bytes);
-
-my ($verbose, $nomail) = CronFns::options();
-my $site = CronFns::site(mySociety::Config::get('BASE_URL'));
-CronFns::language($site);
-
-send_q($site, '4 weeks');
-send_q($site, '26 weeks') if $site eq 'emptyhomes';
-
-# ---
-
-sub send_q {
-    my ($site, $period) = @_;
-
-    # Select all problems that need a questionnaire email sending
-    my $params = {
-        state => [ 'confirmed', 'fixed' ],
-        whensent => [
-            '-and',
-            { '!=', undef },
-            { '<', \"ms_current_timestamp() - '$period'::interval" },
-        ],
-        send_questionnaire => 1,
-    };
-    # FIXME Do these a bit better...
-    if ($site eq 'emptyhomes' && $period eq '4 weeks') {
-        $params->{'(select max(whensent) from questionnaire where me.id=problem_id)'} = undef;
-    } elsif ($site eq 'emptyhomes' && $period eq '26 weeks') {
-        $params->{'(select max(whensent) from questionnaire where me.id=problem_id)'} = { '!=', undef };
-    } else {
-        $params->{'-or'} = [
-            '(select max(whensent) from questionnaire where me.id=problem_id)' => undef,
-            '(select max(whenanswered) from questionnaire where me.id=problem_id)' => { '<', \"ms_current_timestamp() - '$period'::interval" }
-        ];
-    }
-
-    my $unsent = FixMyStreet::App->model('DB::Problem')->search( $params, {
-        order_by => { -desc => 'created' }
-    } );
-
-    while (my $row = $unsent->next) {
-
-        my $cobrand = FixMyStreet::Cobrand->get_class_for_moniker($row->cobrand)->new();
-        $cobrand->set_lang_and_domain($row->lang, 1);
-
-        # Cobranded and non-cobranded messages can share a database. In this case, the conf file 
-        # should specify a vhost to send the reports for each cobrand, so that they don't get sent 
-        # more than once if there are multiple vhosts running off the same database. The email_host
-        # call checks if this is the host that sends mail for this cobrand.
-        next unless $cobrand->email_host;
-
-        my $template;
-        if ($site eq 'emptyhomes') {
-            ($template = $period) =~ s/ //;
-            $template = File::Slurp::read_file("$FindBin::Bin/../templates/email/emptyhomes/" . $row->lang . "/questionnaire-$template.txt");
-        } else {
-            $template = File::Slurp::read_file("$FindBin::Bin/../templates/email/" . $cobrand->moniker . "/questionnaire.txt");
-        }
-
-        my %h = map { $_ => $row->$_ } qw/name title detail category/;
-        $h{created} = Page::prettify_duration( time() - $row->created->epoch, 'week' );
-
-        my $questionnaire = FixMyStreet::App->model('DB::Questionnaire')->create( {
-            problem_id => $row->id,
-            whensent => \'ms_current_timestamp()',
-        } );
-
-        # We won't send another questionnaire unless they ask for it (or it was
-        # the first EHA questionnaire.
-        $row->send_questionnaire( 0 )
-            if $site ne 'emptyhomes' || $period eq '26 weeks';
-
-        my $token = FixMyStreet::App->model("DB::Token")->new_result( {
-            scope => 'questionnaire',
-            data  => $questionnaire->id,
-        } );
-        $h{url} = $cobrand->base_url_for_emails($row->cobrand_data) . '/Q/' . $token->token;
-
-        my $sender = $cobrand->contact_email;
-        my $sender_name = _($cobrand->contact_name);
-        $sender =~ s/team/fms-DO-NOT-REPLY/;
-        my $email = mySociety::Locale::in_gb_locale { mySociety::Email::construct_email({
-            _template_ => $template,
-            _parameters_ => \%h,
-            To => [ [ $row->user->email, $row->name ] ],
-            From => [ $sender, $sender_name ],
-            'Message-ID' => sprintf('<ques-%s-%s@mysociety.org>', time(), unpack('h*', random_bytes(5, 1))),
-        }) };
 
-        print "Sending questionnaire " . $questionnaire->id . ", problem "
-            . $row->id . ", token " . $token->token . " to "
-            . $row->user->email . "\n"
-            if $verbose;
+my %params;
+( $params{verbose}, $params{nomail} ) = CronFns::options();
+$params{site} = CronFns::site(mySociety::Config::get('BASE_URL'));
+CronFns::language($params{site});
 
-        my $result;
-        if ($nomail) {
-            $result = -1;
-        } else {
-            $result = mySociety::EmailUtil::send_email($email, $sender, $row->user->email);
-        }
-        if ($result == mySociety::EmailUtil::EMAIL_SUCCESS) {
-            print "  ...success\n" if $verbose;
-            $row->update();
-            $token->insert();
-        } else {
-            print " ...failed\n" if $verbose;
-            $questionnaire->delete;
-        }
-    }
-}
+FixMyStreet::App->model('DB::Questionnaire')->send_questionnaires( \%params );
 
diff --git a/bin/send-reports b/bin/send-reports
index cec1dee42..feee74c76 100755
--- a/bin/send-reports
+++ b/bin/send-reports
@@ -24,11 +24,8 @@ use FixMyStreet::App;
 use EastHantsWSDL;
 use Utils;
 use mySociety::Config;
-use mySociety::Email;
 use mySociety::EmailUtil;
-use mySociety::Locale;
 use mySociety::MaPit;
-use mySociety::Random qw(random_bytes);
 use mySociety::Web qw(ent);
 
 # Set up site, language etc.
@@ -225,20 +222,17 @@ while (my $row = $unsent->next) {
     my $result = -1;
 
     if ($send_email) {
-
-        my $params = {
-            _template_ => $template,
-            _parameters_ => \%h,
-            To => \@to,
-            From => [ $row->user->email, $row->name ],
-            'Message-ID' => sprintf('<report-%s-%s@mysociety.org>', time(), unpack('h*', random_bytes(5, 1))),
-        };
-        my $email = mySociety::Locale::in_gb_locale { mySociety::Email::construct_email($params) };
-        if (!$nomail) {
-            $result *= mySociety::EmailUtil::send_email($email, mySociety::Config::get('CONTACT_EMAIL'), @recips);
-        } else {
-            print $email;
-        }
+        $result *= FixMyStreet::App->send_email_cron(
+            {
+                _template_ => $template,
+                _parameters_ => \%h,
+                To => \@to,
+                From => [ $row->user->email, $row->name ],
+            },
+            mySociety::Config::get('CONTACT_EMAIL'),
+            \@recips,
+            $nomail
+        );
     }
 
     if ($send_web eq 'easthants') {
diff --git a/perllib/FixMyStreet/App.pm b/perllib/FixMyStreet/App.pm
index 752263d10..10e0dbb28 100644
--- a/perllib/FixMyStreet/App.pm
+++ b/perllib/FixMyStreet/App.pm
@@ -8,6 +8,8 @@ use FixMyStreet::Cobrand;
 use Memcached;
 use Problems;
 use mySociety::Email;
+use mySociety::EmailUtil;
+use mySociety::Random qw(random_bytes);
 use FixMyStreet::Map;
 use FixMyStreet::FakeQ;
 
@@ -308,6 +310,27 @@ sub send_email {
     return $email;
 }
 
+sub send_email_cron {
+    my ( $c, $params, $env_from, $env_to, $nomail ) = @_;
+
+    $params->{'Message-ID'} = sprintf('<fms-cron-%s-%s@mysociety.org>', time(),
+        unpack('h*', random_bytes(5, 1))
+    );
+
+    my $email = mySociety::Locale::in_gb_locale { mySociety::Email::construct_email($params) };
+
+    if ( FixMyStreet->test_mode ) {
+        my $sender = Email::Send->new({ mailer => 'Test' });
+        $sender->send( $email );
+        return 0;
+    } elsif (!$nomail) {
+        return mySociety::EmailUtil::send_email( $email, $env_from, @$env_to );
+    } else {
+        print $email;
+        return 1; # Failure
+    }
+}
+
 =head2 uri_with
 
     $uri = $c->uri_with( ... );
diff --git a/perllib/FixMyStreet/DB/ResultSet/Questionnaire.pm b/perllib/FixMyStreet/DB/ResultSet/Questionnaire.pm
new file mode 100644
index 000000000..80f62f495
--- /dev/null
+++ b/perllib/FixMyStreet/DB/ResultSet/Questionnaire.pm
@@ -0,0 +1,115 @@
+package FixMyStreet::DB::ResultSet::Questionnaire;
+use base 'DBIx::Class::ResultSet';
+
+use strict;
+use warnings;
+use File::Slurp;
+use Page;
+use mySociety::EmailUtil;
+
+sub send_questionnaires {
+    my ( $rs, $params ) = @_;
+    $rs->send_questionnaires_period( '4 weeks', $params );
+    $rs->send_questionnaires_period( '26 weeks', $params )
+        if $params->{site} eq 'emptyhomes';
+}
+
+sub send_questionnaires_period {
+    my ( $rs, $period, $params ) = @_;
+
+    # Select all problems that need a questionnaire email sending
+    my $q_params = {
+        state => [ 'confirmed', 'fixed' ],
+        whensent => [
+            '-and',
+            { '!=', undef },
+            { '<', \"ms_current_timestamp() - '$period'::interval" },
+        ],
+        send_questionnaire => 1,
+    };
+    # FIXME Do these a bit better...
+    if ($params->{site} eq 'emptyhomes' && $period eq '4 weeks') {
+        $q_params->{'(select max(whensent) from questionnaire where me.id=problem_id)'} = undef;
+    } elsif ($params->{site} eq 'emptyhomes' && $period eq '26 weeks') {
+        $q_params->{'(select max(whensent) from questionnaire where me.id=problem_id)'} = { '!=', undef };
+    } else {
+        $q_params->{'-or'} = [
+            '(select max(whensent) from questionnaire where me.id=problem_id)' => undef,
+            '(select max(whenanswered) from questionnaire where me.id=problem_id)' => { '<', \"ms_current_timestamp() - '$period'::interval" }
+        ];
+    }
+
+    my $unsent = FixMyStreet::App->model('DB::Problem')->search( $q_params, {
+        order_by => { -desc => 'confirmed' }
+    } );
+
+    while (my $row = $unsent->next) {
+
+        my $cobrand = FixMyStreet::Cobrand->get_class_for_moniker($row->cobrand)->new();
+        $cobrand->set_lang_and_domain($row->lang, 1);
+
+        # Cobranded and non-cobranded messages can share a database. In this case, the conf file 
+        # should specify a vhost to send the reports for each cobrand, so that they don't get sent 
+        # more than once if there are multiple vhosts running off the same database. The email_host
+        # call checks if this is the host that sends mail for this cobrand.
+        next unless $cobrand->email_host;
+
+        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 );
+        } else {
+            $template = File::Slurp::read_file( FixMyStreet->path_to( "templates/email/" . $cobrand->moniker . "/questionnaire.txt" )->stringify );
+        }
+
+        my %h = map { $_ => $row->$_ } qw/name title detail category/;
+        $h{created} = Page::prettify_duration( time() - $row->confirmed->epoch, 'week' );
+
+        my $questionnaire = FixMyStreet::App->model('DB::Questionnaire')->create( {
+            problem_id => $row->id,
+            whensent => \'ms_current_timestamp()',
+        } );
+
+        # We won't send another questionnaire unless they ask for it (or it was
+        # the first EHA questionnaire.
+        $row->send_questionnaire( 0 )
+            if $params->{site} ne 'emptyhomes' || $period eq '26 weeks';
+
+        my $token = FixMyStreet::App->model("DB::Token")->new_result( {
+            scope => 'questionnaire',
+            data  => $questionnaire->id,
+        } );
+        $h{url} = $cobrand->base_url_for_emails($row->cobrand_data) . '/Q/' . $token->token;
+
+        my $sender = $cobrand->contact_email;
+        my $sender_name = _($cobrand->contact_name);
+        $sender =~ s/team/fms-DO-NOT-REPLY/;
+
+        print "Sending questionnaire " . $questionnaire->id . ", problem "
+            . $row->id . ", token " . $token->token . " to "
+            . $row->user->email . "\n"
+            if $params->{verbose};
+
+        my $result = FixMyStreet::App->send_email_cron(
+            {
+                _template_ => $template,
+                _parameters_ => \%h,
+                To => [ [ $row->user->email, $row->name ] ],
+                From => [ $sender, $sender_name ],
+            },
+            $sender,
+            [ $row->user->email ],
+            $params->{nomail}
+        );
+        if ($result == mySociety::EmailUtil::EMAIL_SUCCESS) {
+            print "  ...success\n" if $params->{verbose};
+            $row->update();
+            $token->insert();
+        } else {
+            print " ...failed\n" if $params->{verbose};
+            $questionnaire->delete;
+        }
+    }
+}
+
+1;
diff --git a/t/app/controller/questionnaire.t b/t/app/controller/questionnaire.t
index af14332d4..a245be343 100644
--- a/t/app/controller/questionnaire.t
+++ b/t/app/controller/questionnaire.t
@@ -45,29 +45,29 @@ my $report = FixMyStreet::App->model('DB::Problem')->find_or_create(
 my $report_id = $report->id;
 ok $report, "created test report - $report_id";
 
-# FIXME Should call send-questionnaires here to test that is working okay.
-# For now, just generate the questionnaire etc. manually.
-# Call send-questionnaires (for normal non-EHA site)
-# my $email = $mech->get_email;
-# ok $email, "got an email";
-# like $email->body, qr/fill in our short questionnaire/i, "got questionnaire email";
-# my ($url) = $email->body =~ m{(http://\S+)};
-# ok $url, "extracted questionnaire url '$url'";
+# Call the questionaire sending function...
+FixMyStreet::App->model('DB::Questionnaire')->send_questionnaires( {
+    site => 'fixmystreet'
+} );
+my $email = $mech->get_email;
+ok $email, "got an email";
+like $email->body, qr/fill in our short questionnaire/i, "got questionnaire email";
+my ($token) = $email->body =~ m{http://.*?/Q/(\S+)};
+ok $token, "extracted questionnaire token '$token'";
+$mech->clear_emails_ok;
 
-$report->send_questionnaire( 0 );
-$report->update;
+$report->discard_changes;
+is $report->send_questionnaire, 0;
 
-my $questionnaire = FixMyStreet::App->model('DB::Questionnaire')->find_or_create(
-    {
-        problem_id => $report->id,
-        whensent => '2011-04-01 12:00:00',
-    }
-);
-ok $questionnaire, 'added questionnaire';
-my $token = FixMyStreet::App->model("DB::Token")->create(
-    { scope => 'questionnaire', data => $questionnaire->id }
-);
-ok $token, 'added token for questionnaire';
+$token = FixMyStreet::App->model("DB::Token")->find( {
+    scope => 'questionnaire', token => $token
+} );
+ok $token, 'found token for questionnaire';
+
+my $questionnaire = FixMyStreet::App->model('DB::Questionnaire')->find( {
+    id => $token->data
+} );
+ok $questionnaire, 'found questionnaire';
 
 foreach my $test (
     {
@@ -251,7 +251,21 @@ foreach my $test (
 
 # EHA extra checking
 ok $mech->host("reportemptyhomes.com"), 'change host to reportemptyhomes';
-$mech->get_ok("/Q/" . $token->token);
+
+# Reset, and all the questionaire sending function - FIXME should it detect site itself somehow?
+$report->send_questionnaire( 1 );
+$report->update;
+$questionnaire->delete;
+FixMyStreet::App->model('DB::Questionnaire')->send_questionnaires( {
+    site => 'emptyhomes'
+} );
+$email = $mech->get_email;
+ok $email, "got an email";
+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'";
+
+$mech->get_ok("/Q/" . $token);
 $mech->content_contains( 'should have reported what they have done' );
 
 # Test already answered the ever reported question, so not shown again
@@ -264,14 +278,14 @@ my $questionnaire2 = FixMyStreet::App->model('DB::Questionnaire')->find_or_creat
 );
 ok $questionnaire, 'added another questionnaire';
 ok $mech->host("fixmystreet.com"), 'change host to fixmystreet';
-$mech->get_ok("/Q/" . $token->token);
+$mech->get_ok("/Q/" . $token);
 $mech->title_like( qr/Questionnaire/ );
 $mech->content_contains( 'Has this problem been fixed?' );
 $mech->content_lacks( 'ever reported' );
 
 # EHA extra checking
 ok $mech->host("reportemptyhomes.com"), 'change host to reportemptyhomes';
-$mech->get_ok("/Q/" . $token->token);
+$mech->get_ok("/Q/" . $token);
 $mech->content_contains( 'made a lot of progress' );
 
 $mech->delete_user('test@example.com');