Fixes and improvements for tests.

6 files changed, 114 insertions, 94 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index bbfa8170f..998cb83a8 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -340,7 +340,7 @@ sub update_contacts : Private {
     } elsif ( $posted eq 'open311' ) {
         $c->forward('check_token');
 
-        my %params = map { $_ => $c->req->param($_) } qw/open311_id endpoint jurisdiction api_key area_id send_method send_comments suppress_alerts comment_user_id/;
+        my %params = map { $_ => $c->req->param($_) || '' } qw/open311_id endpoint jurisdiction api_key area_id send_method send_comments suppress_alerts comment_user_id/;
 
         if ( $params{open311_id} ) {
             my $conf = $c->model('DB::Open311Conf')->find( { id => $params{open311_id} } );
@@ -349,8 +349,8 @@ sub update_contacts : Private {
             $conf->jurisdiction( $params{jurisdiction} );
             $conf->api_key( $params{api_key} );
             $conf->send_method( $params{send_method} );
-            $conf->send_comments( $params{send_comments} );
-            $conf->suppress_alerts( $params{suppress_alerts} );
+            $conf->send_comments( $params{send_comments} || 0);
+            $conf->suppress_alerts( $params{suppress_alerts} || 0);
             $conf->comment_user_id( $params{comment_user_id} || undef );
 
             $conf->update();
@@ -363,8 +363,8 @@ sub update_contacts : Private {
             $conf->jurisdiction( $params{jurisdiction} );
             $conf->api_key( $params{api_key} );
             $conf->send_method( $params{send_method} );
-            $conf->send_comments( $params{send_comments} );
-            $conf->suppress_alerts( $params{suppress_alerts} );
+            $conf->send_comments( $params{send_comments} || 0);
+            $conf->suppress_alerts( $params{suppress_alerts} || 0);
             $conf->comment_user_id( $params{comment_user_id} || undef );
 
             $conf->insert();
diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm
index 8be513b08..7542fbe96 100644
--- a/perllib/FixMyStreet/App/Controller/Report/Update.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm
@@ -105,8 +105,10 @@ sub process_user : Private {
         my $name = scalar $c->req->param('name');
         $user->name( Utils::trim_text( $name ) ) if $name;
         my $title = scalar $c->req->param('fms_extra_title');
-        $c->log->debug( 'user exists and title is ' . $title );
-        $user->title( Utils::trim_text( $title ) ) if $title;
+        if ( $title ) {
+            $c->log->debug( 'user exists and title is ' . $title );
+            $user->title( Utils::trim_text( $title ) );
+        }
         $update->user( $user );
         return 1;
     }
diff --git a/t/app/controller/alert_new.t b/t/app/controller/alert_new.t
index 3a4c2ef81..85eea7cda 100644
--- a/t/app/controller/alert_new.t
+++ b/t/app/controller/alert_new.t
@@ -445,6 +445,7 @@ subtest "Test normal alert signups and that alerts are sent" => sub {
     ok $update, "created test update - $update_id";
 
     FixMyStreet::App->model('DB::AlertType')->email_alerts();
+    # TODO Note the below will fail if the db has an existing alert that matches
     $mech->email_count_is(3);
     my @emails = $mech->get_email;
     my $count;
diff --git a/t/app/controller/questionnaire.t b/t/app/controller/questionnaire.t
index 0e2a71184..2d4fdb711 100644
--- a/t/app/controller/questionnaire.t
+++ b/t/app/controller/questionnaire.t
@@ -322,67 +322,79 @@ for my $test (
     };
 }
 
-# EHA extra checking
-ok $mech->host("reportemptyhomes.com"), 'change host to reportemptyhomes';
-
-# 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";
-$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'";
-
-$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
-$dt = $dt->add( weeks => 4 );
-my $questionnaire2 = FixMyStreet::App->model('DB::Questionnaire')->find_or_create(
-    {
-        problem_id => $report->id,
-        whensent => $dt->ymd . ' ' . $dt->hms,
-        ever_reported => 1,
-    }
-);
-ok $questionnaire2, 'added another questionnaire';
-ok $mech->host("fixmystreet.com"), 'change host to fixmystreet';
-$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);
-$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;
+SKIP: {
+    skip( "Need 'emptyhomes' in ALLOWED_COBRANDS config", 18 )
+        unless FixMyStreet::Cobrand->exists('emptyhomes');
+
+    # EHA extra checking
+    ok $mech->host("reportemptyhomes.com"), 'change host to reportemptyhomes';
+
+    # 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";
+    $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'";
+
+    $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
+    $dt = $dt->add( weeks => 4 );
+    my $questionnaire2 = FixMyStreet::App->model('DB::Questionnaire')->find_or_create(
+        {
+            problem_id => $report->id,
+            whensent => $dt->ymd . ' ' . $dt->hms,
+            ever_reported => 1,
+        }
+    );
+    ok $questionnaire2, 'added another questionnaire';
+    ok $mech->host("fixmystreet.com"), 'change host to fixmystreet';
+    $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);
+    $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';
+
+    $questionnaire2->delete;
+}
 
-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';
+SKIP: {
+    skip( "Need 'fiksgatami' in ALLOWED_COBRANDS config", 5 )
+        unless FixMyStreet::Cobrand->exists('fiksgatami');
+
+    # I18N Unicode extra testing using FiksGataMi
+    $report->send_questionnaire( 1 );
+    $report->cobrand( 'fiksgatami' );
+    $report->update;
+    $questionnaire->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/app/uri_for.t b/t/app/uri_for.t
index 51a6e8a0e..eecf30e32 100644
--- a/t/app/uri_for.t
+++ b/t/app/uri_for.t
@@ -78,31 +78,36 @@ is(
     'FiksGataMi url with lat not zoom'
 );
 
-like(
-    $reh_en_c->uri_for_email( '/foo' ),
-    qr{^http://en.},
-    'adds en to retain language'
-);
-
-# instantiate this here otherwise sets locale to cy and breaks test
-# above
-my $reh_cy_c = FixMyStreet::App->new(
-    {
-        request => Catalyst::Request->new(
-            {
-                base => URI->new('http://cy.reportemptyhomes.com/'),
-                uri  => URI->new('http://cy.reportemptyhomes.com/test_namespace')
-            }
-        ),
-        namespace => 'test_namespace',
-    }
-);
-$reh_cy_c->setup_request();
-
-like(
-    $reh_cy_c->uri_for_email( '/foo' ),
-    qr{^http://cy.},
-    'retains language'
-);
+SKIP: {
+    skip( "Need 'emptyhomes' in ALLOWED_COBRANDS config", 2 )
+        unless FixMyStreet::Cobrand->exists('emptyhomes');
+
+    like(
+        $reh_en_c->uri_for_email( '/foo' ),
+        qr{^http://en.},
+        'adds en to retain language'
+    );
+
+    # instantiate this here otherwise sets locale to cy and breaks test
+    # above
+    my $reh_cy_c = FixMyStreet::App->new(
+        {
+            request => Catalyst::Request->new(
+                {
+                    base => URI->new('http://cy.reportemptyhomes.com/'),
+                    uri  => URI->new('http://cy.reportemptyhomes.com/test_namespace')
+                }
+            ),
+            namespace => 'test_namespace',
+        }
+    );
+    $reh_cy_c->setup_request();
+
+    like(
+        $reh_cy_c->uri_for_email( '/foo' ),
+        qr{^http://cy.},
+        'retains language'
+    );
+}
 
 done_testing();
diff --git a/templates/web/default/admin/council_contacts.html b/templates/web/default/admin/council_contacts.html
index dac73cbdd..e35c8cda2 100644
--- a/templates/web/default/admin/council_contacts.html
+++ b/templates/web/default/admin/council_contacts.html
@@ -137,7 +137,7 @@
 
     <p>
         <label for"comment_user_id">User to attribute fetched comments to</label>:
-        <input type="textbox" name="comment_user_id" value="[% conf.comment_user_id %]">
+        <input type="text" name="comment_user_id" value="[% conf.comment_user_id %]">
     </p>
 
     <p>