Allow ‘report as another user’ to only provide a phone number

8 files changed, 96 insertions, 10 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 065ad72e4..51dcc36c4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@
         - Ask for current password/send email on password change. #1974
         - Add minimum password length and common password checking.
         - Nicer display of national phone numbers.
+        - 'Report as another user' allows phone number without email. #1978
     - Bugfixes:
         - Fix bug specifying category in URL on /around. #1950
         - Fix bug with multiple select-multiples on a page. #1951
diff --git a/conf/general.yml-example b/conf/general.yml-example
index 79af1c78e..b9900dea2 100644
--- a/conf/general.yml-example
+++ b/conf/general.yml-example
@@ -206,9 +206,12 @@ TESTING_COUNCILS: ''
 # if you're using Message Manager, include the URL here (see https://github.com/mysociety/message-manager/)
 MESSAGE_MANAGER_URL: ''
 
+# If you want to use SMS login or 'report as' with just a phone number, you'll
+# need to set the site's two-digit ISO 3166 country code (e.g. GB) here.
+PHONE_COUNTRY: ''
+
 # If you enable login via SMS authentication, you'll need a twilio account
 SMS_AUTHENTICATION: 0
-PHONE_COUNTRY: ''
 TWILIO_ACCOUNT_SID: ''
 TWILIO_AUTH_TOKEN: ''
 TWILIO_FROM_PARAMETER: ''
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index 85652450e..eff45013f 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -790,12 +790,20 @@ sub process_user : Private {
         return 1;
     } }
 
+    if ( $c->stash->{contributing_as_another_user} && !$params{username} ) {
+        # If the 'username' (i.e. email) field is blank, then use the phone
+        # field for the username.
+        $params{username} = $params{phone};
+    }
+
     my $parsed = FixMyStreet::SMS->parse_username($params{username});
     my $type = $parsed->{type} || 'email';
-    $type = 'email' unless FixMyStreet->config('SMS_AUTHENTICATION');
+    $type = 'email' unless FixMyStreet->config('SMS_AUTHENTICATION') || $c->stash->{contributing_as_another_user};
     $report->user( $c->model('DB::User')->find_or_new( { $type => $parsed->{username} } ) )
         unless $report->user;
 
+    $c->stash->{phone_may_be_mobile} = $type eq 'phone' && $parsed->{may_be_mobile};
+
     # The user is trying to sign in. We only care about username from the params.
     if ( $c->get_param('submit_sign_in') || $c->get_param('password_sign_in') ) {
         $c->stash->{tfa_data} = {
@@ -1076,6 +1084,12 @@ sub check_for_errors : Private {
         delete $field_errors{username};
     }
 
+    # if we're contributing as someone else then allow landline numbers
+    if ( $field_errors{phone} && $c->stash->{contributing_as_another_user} && !$c->stash->{phone_may_be_mobile}) {
+        delete $field_errors{username};
+        delete $field_errors{phone};
+    }
+
     # add the photo error if there is one.
     if ( my $photo_error = delete $c->stash->{photo_error} ) {
         $field_errors{photo} = $photo_error;
@@ -1384,7 +1398,7 @@ sub redirect_or_confirm_creation : Private {
     if ( $report->confirmed ) {
         # Subscribe problem reporter to email updates
         $c->forward( 'create_reporter_alert' );
-        if ($c->stash->{contributing_as_another_user}) {
+        if ($c->stash->{contributing_as_another_user} && $report->user->email) {
             $c->send_email( 'other-reported.txt', {
                 to => [ [ $report->user->email, $report->name ] ],
             } );
diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm
index a6b927ad1..db68236bf 100644
--- a/perllib/FixMyStreet/DB/Result/User.pm
+++ b/perllib/FixMyStreet/DB/Result/User.pm
@@ -200,9 +200,13 @@ sub check_for_errors {
     } elsif ($self->phone_verified) {
         my $parsed = FixMyStreet::SMS->parse_username($self->phone);
         if (!$parsed->{phone}) {
+            # Errors with the phone number may apply to both the username or
+            # phone field depending on the form.
             $errors{username} = _('Please check your phone number is correct');
+            $errors{phone} = _('Please check your phone number is correct');
         } elsif (!$parsed->{may_be_mobile}) {
             $errors{username} = _('Please enter a mobile number');
+            $errors{phone} = _('Please enter a mobile number');
         }
     }
 
diff --git a/perllib/FixMyStreet/Script/Reports.pm b/perllib/FixMyStreet/Script/Reports.pm
index 7e14fbb1e..d6a614651 100644
--- a/perllib/FixMyStreet/Script/Reports.pm
+++ b/perllib/FixMyStreet/Script/Reports.pm
@@ -106,10 +106,6 @@ sub send(;$) {
              $row->user->email eq $cobrand->anonymous_account->{'email'}
          ) {
             $h{anonymous_report} = 1;
-            $h{user_details} = _('This report was submitted anonymously');
-        } else {
-            $h{user_details} = sprintf(_('Name: %s'), $row->name) . "\n\n";
-            $h{user_details} .= sprintf(_('Email: %s'), $row->user->email) . "\n\n";
         }
 
         $cobrand->call_hook(process_additional_metadata_for_email => $row, \%h);
diff --git a/t/app/controller/report_as_other.t b/t/app/controller/report_as_other.t
index 91644e8ce..22bd5185e 100644
--- a/t/app/controller/report_as_other.t
+++ b/t/app/controller/report_as_other.t
@@ -39,7 +39,7 @@ subtest "Body user, has permission to add report as council" => sub {
 };
 
 my @users;
-subtest "Body user, has permission to add report as another user" => sub {
+subtest "Body user, has permission to add report as another user with email" => sub {
     my $report = add_report(
         'contribute_as_another_user',
         form_as => 'another_user',
@@ -57,7 +57,49 @@ subtest "Body user, has permission to add report as another user" => sub {
     push @users, $report->user;
 };
 
-subtest "Body user, has permission to add report as another (existing) user" => sub {
+subtest "Body user, has permission to add report as another user with mobile phone number" => sub {
+    my $report = add_report(
+        'contribute_as_another_user',
+        form_as => 'another_user',
+        title => "Test Report",
+        detail => 'Test report details.',
+        category => 'Potholes',
+        name => 'Another User',
+        username => '07906 111111',
+    );
+    is $report->name, 'Another User', 'report name is given name';
+    is $report->user->name, 'Another User', 'user name matches';
+    is $report->user->phone, '+447906111111', 'user phone correct';
+    is $report->user->phone_verified, 1, 'user phone verified';
+    is $report->user->email, undef, 'user email correct';
+    is $report->user->email_verified, 0, 'user email not verified';
+    isnt $report->user->id, $user->id, 'user does not match';
+    $mech->email_count_is(0);
+    push @users, $report->user;
+};
+
+subtest "Body user, has permission to add report as another user with landline number" => sub {
+    my $report = add_report(
+        'contribute_as_another_user',
+        form_as => 'another_user',
+        title => "Test Report",
+        detail => 'Test report details.',
+        category => 'Potholes',
+        name => 'Another User',
+        username => '01685 222222',
+    );
+    is $report->name, 'Another User', 'report name is given name';
+    is $report->user->name, 'Another User', 'user name matches';
+    is $report->user->phone, '+441685222222', 'user phone correct';
+    is $report->user->phone_verified, 1, 'user phone verified';
+    is $report->user->email, undef, 'user email correct';
+    is $report->user->email_verified, 0, 'user email not verified';
+    isnt $report->user->id, $user->id, 'user does not match';
+    $mech->email_count_is(0);
+    push @users, $report->user;
+};
+
+subtest "Body user, has permission to add report as another (existing) user with email" => sub {
     $mech->create_user_ok('existing@example.net', name => 'Existing User');
     my $report = add_report(
         'contribute_as_another_user',
@@ -76,6 +118,25 @@ subtest "Body user, has permission to add report as another (existing) user" =>
     push @users, $report->user;
 };
 
+subtest "Body user, has permission to add report as another (existing) user with phone" => sub {
+    $mech->create_user_ok('+447906333333', name => 'Existing User');
+    my $report = add_report(
+        'contribute_as_another_user',
+        form_as => 'another_user',
+        title => "Test Report",
+        detail => 'Test report details.',
+        category => 'Potholes',
+        name => 'Existing Yooser',
+        username => '07906 333333',
+    );
+    is $report->name, 'Existing Yooser', 'report name is given name';
+    is $report->user->name, 'Existing User', 'user name remains same';
+    is $report->user->phone, '+447906333333', 'user phone correct';
+    isnt $report->user->id, $user->id, 'user does not match';
+    $mech->email_count_is(0);
+    push @users, $report->user;
+};
+
 subtest "Body user, has permission to add report as anonymous user" => sub {
     my $report = add_report(
         'contribute_as_anonymous_user',
@@ -155,6 +216,7 @@ sub start_report {
     FixMyStreet::override_config {
         ALLOWED_COBRANDS => [ 'fixmystreet' ],
         MAPIT_URL => 'http://mapit.uk/',
+        PHONE_COUNTRY => 'GB',
     }, sub {
         $mech->get_ok('/report/new?latitude=51.7549262252&longitude=-1.25617899435');
     };
@@ -166,6 +228,7 @@ sub add_report {
     FixMyStreet::override_config {
         ALLOWED_COBRANDS => [ 'fixmystreet' ],
         MAPIT_URL => 'http://mapit.uk/',
+        PHONE_COUNTRY => 'GB',
     }, sub {
         dropdown_shown(1);
         $mech->submit_form_ok({
diff --git a/templates/web/base/report/new/form_user_loggedin.html b/templates/web/base/report/new/form_user_loggedin.html
index 04a93ef74..ad74a5654 100644
--- a/templates/web/base/report/new/form_user_loggedin.html
+++ b/templates/web/base/report/new/form_user_loggedin.html
@@ -35,7 +35,7 @@
 [% END %]
 
 [% IF c.user.email_verified %]
-    <label for="form_username">[% loc('Email address') %]</label>
+    <label for="form_username">[% loc('Email address') %]<span class="hidden"> [% loc('(optional)') %]</span></label>
     <input class="form-control" id="form_username" name="username"
     [%- IF NOT can_contribute_as_another_user -%]
     disabled
diff --git a/web/cobrands/fixmystreet/staff.js b/web/cobrands/fixmystreet/staff.js
index e99af3c6b..f2b98744b 100644
--- a/web/cobrands/fixmystreet/staff.js
+++ b/web/cobrands/fixmystreet/staff.js
@@ -172,30 +172,35 @@ $.extend(fixmystreet.set_up, {
             val = opt.value,
             txt = opt.text;
         var $emailInput = $('input[name=username]');
+        var $emailOptionalLabel = $('label[for=form_username] span');
         var $nameInput = $('input[name=name]');
         var $phoneInput = $('input[name=phone]');
         var $showNameCheckbox = $('input[name=may_show_name]');
         var $addAlertCheckbox = $('#form_add_alert');
         if (val === 'myself') {
             $emailInput.val($emailInput.prop('defaultValue')).prop('disabled', true);
+            $emailOptionalLabel.addClass('hidden');
             $nameInput.val($nameInput.prop('defaultValue')).prop('disabled', false);
             $phoneInput.val($phoneInput.prop('defaultValue')).prop('disabled', false);
             $showNameCheckbox.prop('checked', false).prop('disabled', false);
             $addAlertCheckbox.prop('checked', true).prop('disabled', false);
         } else if (val === 'another_user') {
             $emailInput.val('').prop('disabled', false);
+            $emailOptionalLabel.removeClass('hidden');
             $nameInput.val('').prop('disabled', false);
             $phoneInput.val('').prop('disabled', false);
             $showNameCheckbox.prop('checked', false).prop('disabled', true);
             $addAlertCheckbox.prop('checked', true).prop('disabled', false);
         } else if (val === 'anonymous_user') {
             $emailInput.val('-').prop('disabled', true);
+            $emailOptionalLabel.addClass('hidden');
             $nameInput.val('-').prop('disabled', true);
             $phoneInput.val('-').prop('disabled', true);
             $showNameCheckbox.prop('checked', false).prop('disabled', true);
             $addAlertCheckbox.prop('checked', false).prop('disabled', true);
         } else if (val === 'body') {
             $emailInput.val('-').prop('disabled', true);
+            $emailOptionalLabel.addClass('hidden');
             $nameInput.val(txt).prop('disabled', true);
             $phoneInput.val('-').prop('disabled', true);
             $showNameCheckbox.prop('checked', true).prop('disabled', true);