diff options
34 files changed, 1181 insertions, 332 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index fec83feae..c5a2bec22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,8 @@ * Unreleased - New features: - - Optional logging in using confirmation by phone text. + - Optional verification of reports and updates, and logging in, + using confirmation by phone text. - Area summary statistics page in admin #1834 - Bugfixes - Shortlist menu item always remains a link #1855 diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index c2fd2a377..fa3967bf3 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -13,6 +13,7 @@ use Path::Class; use Utils; use mySociety::EmailUtil; use JSON::MaybeXS; +use FixMyStreet::SMS; =head1 NAME @@ -116,19 +117,25 @@ sub report_new : Path : Args(0) { $c->forward('redirect_or_confirm_creation'); } -# This is for the new phonegap versions of the app. It looks a lot like -# report_new but there's a few workflow differences as we only ever want -# to sent JSON back here - sub report_new_test : Path('_test_') : Args(0) { my ( $self, $c ) = @_; $c->stash->{template} = 'email_sent.html'; $c->stash->{email_type} = $c->get_param('email_type'); } +# This is for the new phonegap versions of the app. It looks a lot like +# report_new but there's a few workflow differences as we only ever want +# to sent JSON back here + sub report_new_ajax : Path('mobile') : Args(0) { my ( $self, $c ) = @_; + # Apps are sending email as username + # Prepare for when they upgrade + if (!$c->get_param('username')) { + $c->set_param('username', $c->get_param('email')); + } + # create the report - loading a partial if available $c->forward('initialize_report'); @@ -737,14 +744,12 @@ sub process_user : Private { # Extract all the params to a hash to make them easier to work with my %params = map { $_ => $c->get_param($_) } - ( 'email', 'name', 'phone', 'password_register', 'fms_extra_title' ); - - my $user_title = Utils::trim_text( $params{fms_extra_title} ); + ( 'username', 'email', 'name', 'phone', 'password_register', 'fms_extra_title' ); if ( $c->cobrand->allow_anonymous_reports ) { my $anon_details = $c->cobrand->anonymous_account; - for my $key ( qw( email name ) ) { + for my $key ( qw( username email name ) ) { $params{ $key } ||= $anon_details->{ $key }; } } @@ -759,34 +764,29 @@ sub process_user : Private { last; } - $user->name( Utils::trim_text( $params{name} ) ) if $params{name}; - $user->phone( Utils::trim_text( $params{phone} ) ); - $user->title( $user_title ) if $user_title; $report->user( $user ); + $c->forward('update_user', [ \%params ]); if ($c->stash->{contributing_as_body} = $user->contributing_as('body', $c, $c->stash->{bodies}) or $c->stash->{contributing_as_anonymous_user} = $user->contributing_as('anonymous_user', $c, $c->stash->{bodies})) { $report->name($user->from_body->name); $user->name($user->from_body->name) unless $user->name; $c->stash->{no_reporter_alert} = 1; - } else { - $report->name($user->name); } return 1; } } - # cleanup the email address - my $email = $params{email} ? lc $params{email} : ''; - $email =~ s{\s+}{}g; - - $report->user( $c->model('DB::User')->find_or_new( { email => $email } ) ) + my $parsed = FixMyStreet::SMS->parse_username($params{username}); + my $type = $parsed->{type} || 'email'; + $type = 'email' unless FixMyStreet->config('SMS_AUTHENTICATION'); + $report->user( $c->model('DB::User')->find_or_new( { $type => $parsed->{username} } ) ) unless $report->user; - # The user is trying to sign in. We only care about email from the params. + # 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') ) { - unless ( $c->forward( '/auth/sign_in', [ $email ] ) ) { - $c->stash->{field_errors}->{password} = _('There was a problem with your email/password combination. If you cannot remember your password, or do not have one, please fill in the ‘sign in by email’ section of the form.'); + unless ( $c->forward( '/auth/sign_in', [ $params{username} ] ) ) { + $c->stash->{field_errors}->{password} = _('There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the ‘No’ section of the form.'); return 1; } my $user = $c->user->obj; @@ -798,17 +798,28 @@ sub process_user : Private { return 1; } - # set the user's name, phone, and password - $report->user->name( Utils::trim_text( $params{name} ) ) if $params{name}; - $report->user->phone( Utils::trim_text( $params{phone} ) ); + $c->forward('update_user', [ \%params ]); $report->user->password( Utils::trim_text( $params{password_register} ) ) if $params{password_register}; - $report->user->title( $user_title ) if $user_title; - $report->name( Utils::trim_text( $params{name} ) ); return 1; } +sub update_user : Private { + my ($self, $c, $params) = @_; + my $report = $c->stash->{report}; + my $user = $report->user; + $user->name( Utils::trim_text( $params->{name} ) ); + $report->name($user->name); + if (!$user->phone_verified) { + $user->phone( Utils::trim_text( $params->{phone} ) ); + } elsif (!$user->email_verified) { + $user->email( Utils::trim_text( $params->{email} ) ); + } + my $user_title = Utils::trim_text( $params->{fms_extra_title} ); + $user->title( $user_title ) if $user_title; +} + =head2 process_report Looking at the parameters passed in create a new item and return it. Does not @@ -1031,11 +1042,11 @@ sub check_for_errors : Private { delete $field_errors{name}; } - # if using social login then we don't care about name and email errors + # if using social login then we don't care about other errors $c->stash->{is_social_user} = $c->get_param('facebook_sign_in') || $c->get_param('twitter_sign_in'); if ( $c->stash->{is_social_user} ) { delete $field_errors{name}; - delete $field_errors{email}; + delete $field_errors{username}; } # add the photo error if there is one. @@ -1056,7 +1067,8 @@ sub tokenize_user : Private { my ($self, $c, $report) = @_; $c->stash->{token_data} = { name => $report->user->name, - phone => $report->user->phone, + (!$report->user->phone_verified ? (phone => $report->user->phone) : ()), + (!$report->user->email_verified ? (email => $report->user->email) : ()), password => $report->user->password, title => $report->user->title, }; @@ -1089,6 +1101,114 @@ sub send_problem_confirm_email : Private { } ); } +sub send_problem_confirm_text : Private { + my ( $self, $c ) = @_; + my $data = $c->stash->{token_data} || {}; + my $report = $c->stash->{report}; + + $data->{id} = $report->id; + $c->forward('/auth/phone/send_token', [ $data, 'problem', $report->user->phone ]); + $c->stash->{submit_url} = '/report/new/text'; +} + +sub confirm_by_text : Path('text') { + my ( $self, $c ) = @_; + + my $token = $c->stash->{token} = $c->get_param('token'); + my $code = $c->get_param('code') || ''; + + my $data = $c->stash->{token_data} = $c->forward('/auth/get_token', [ $token, 'problem' ]) || return; + if ($data->{code} ne $code) { + $c->stash->{template} = 'auth/smsform.html'; + $c->stash->{submit_url} = '/report/new/text'; + $c->stash->{incorrect_code} = 1; + return; + } + + $c->detach('process_confirmation'); +} + +sub process_confirmation : Private { + my ( $self, $c ) = @_; + + $c->stash->{template} = 'tokens/confirm_problem.html'; + my $data = $c->stash->{token_data}; + + unless ($c->stash->{report}) { + # Look at all problems, not just cobrand, in case am approving something we don't actually show + $c->stash->{report} = $c->model('DB::Problem')->find({ id => $data->{id} }) || return; + } + my $problem = $c->stash->{report}; + + # check that this email or domain are not the cause of abuse. If so hide it. + if ( $problem->is_from_abuser ) { + $problem->update( + { state => 'hidden', lastupdate => \'current_timestamp' } ); + $c->stash->{template} = 'tokens/abuse.html'; + return; + } + + # For Zurich, email confirmation simply sets a flag, it does not change the + # problem state, log in, or anything else + if ($c->cobrand->moniker eq 'zurich') { + $problem->set_extra_metadata( email_confirmed => 1 ); + $problem->update( { + confirmed => \'current_timestamp', + } ); + + if ( $data->{name} || $data->{password} ) { + $problem->user->name( $data->{name} ) if $data->{name}; + $problem->user->phone( $data->{phone} ) if $data->{phone}; + $problem->user->update; + } + + return 1; + } + + if ($problem->state ne 'unconfirmed') { + my $report_uri = $c->cobrand->base_url_for_report( $problem ) . $problem->url; + $c->res->redirect($report_uri); + return; + } + + # We have an unconfirmed problem + $problem->update( + { + state => 'confirmed', + confirmed => \'current_timestamp', + lastupdate => \'current_timestamp', + } + ); + + # Subscribe problem reporter to email updates + $c->forward( '/report/new/create_reporter_alert' ); + + # log the problem creation user in to the site + if ( $data->{name} || $data->{password} ) { + if (!$problem->user->email_verified) { + $problem->user->email( $data->{email} ) if $data->{email}; + } elsif (!$problem->user->phone_verified) { + $problem->user->phone( $data->{phone} ) if $data->{phone}; + } + $problem->user->password( $data->{password}, 1 ) if $data->{password}; + for (qw(name title facebook_id twitter_id)) { + $problem->user->$_( $data->{$_} ) if $data->{$_}; + } + $problem->user->update; + } + if ($problem->user->email_verified) { + $c->authenticate( { email => $problem->user->email, email_verified => 1 }, 'no_password' ); + } elsif ($problem->user->phone_verified) { + $c->authenticate( { phone => $problem->user->phone, phone_verified => 1 }, 'no_password' ); + } else { + warn "Reached user authentication with no username verification"; + } + $c->set_session_cookie_expire(0); + + $c->stash->{created_report} = 'fromemail'; + return 1; +} + =head2 save_user_and_report Save the user and the report. @@ -1143,11 +1263,7 @@ sub save_user_and_report : Private { # Save or update the user if appropriate if ( $c->cobrand->never_confirm_reports ) { - if ( $report->user->in_storage() ) { - $report->user->update(); - } else { - $report->user->insert(); - } + $report->user->update_or_insert; $report->confirm(); } elsif ( $c->forward('created_as_someone_else', [ $c->stash->{bodies} ]) ) { # If created on behalf of someone else, we automatically confirm it, @@ -1157,7 +1273,11 @@ sub save_user_and_report : Private { # User does not exist. $c->forward('tokenize_user', [ $report ]); $report->user->name( undef ); - $report->user->phone( undef ); + if (!$report->user->email_verified) { + $report->user->email( undef ); + } elsif (!$report->user->phone_verified) { + $report->user->phone( undef ); + } $report->user->password( '', 1 ); $report->user->title( undef ); $report->user->insert(); @@ -1177,8 +1297,7 @@ sub save_user_and_report : Private { $c->log->info($report->user->id . ' exists, but is not logged in for this report'); } - # save the report; - $report->in_storage ? $report->update : $report->insert(); + $report->update_or_insert; # tidy up if ( my $token = $c->stash->{partial_token} ) { @@ -1264,13 +1383,20 @@ sub redirect_or_confirm_creation : Private { return 1; } - # otherwise email a confirm token to them. - $c->forward( 'send_problem_confirm_email' ); - - # tell user that they've been sent an email - $c->stash->{template} = 'email_sent.html'; - $c->stash->{email_type} = 'problem'; - $c->log->info($report->user->id . ' created ' . $report->id . ', email sent, ' . ($c->stash->{token_data}->{password} ? 'password set' : 'password not set')); + # otherwise email or text a confirm token to them. + my $thing = 'email'; + if ($report->user->email_verified) { + $c->forward( 'send_problem_confirm_email' ); + # tell user that they've been sent an email + $c->stash->{template} = 'email_sent.html'; + $c->stash->{email_type} = 'problem'; + } elsif ($report->user->phone_verified) { + $c->forward( 'send_problem_confirm_text' ); + $thing = 'text'; + } else { + warn "Reached problem confirmation with no username verification"; + } + $c->log->info($report->user->id . ' created ' . $report->id . ", $thing sent, " . ($c->stash->{token_data}->{password} ? 'password set' : 'password not set')); } sub create_reporter_alert : Private { diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm index 36c45e621..66724f2d1 100644 --- a/perllib/FixMyStreet/App/Controller/Report/Update.pm +++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm @@ -36,18 +36,6 @@ sub report_update : Path : Args(0) { $c->forward('redirect_or_confirm_creation'); } -sub confirm : Private { - my ( $self, $c ) = @_; - - $c->stash->{update}->confirm; - $c->stash->{update}->update; - - $c->forward('update_problem'); - $c->forward('signup_for_alerts'); - - return 1; -} - sub update_problem : Private { my ( $self, $c ) = @_; @@ -109,6 +97,10 @@ sub process_user : Private { my $update = $c->stash->{update}; + # Extract all the params to a hash to make them easier to work with + my %params = map { $_ => $c->get_param($_) } + ( 'username', 'name', 'password_register', 'fms_extra_title' ); + # Extra block to use 'last' if ( $c->user_exists ) { { my $user = $c->user->obj; @@ -118,13 +110,9 @@ sub process_user : Private { last; } - my $name = $c->get_param('name'); - $user->name( Utils::trim_text( $name ) ) if $name; - my $title = $c->get_param('fms_extra_title'); - if ( $title ) { - $c->log->debug( 'user exists and title is ' . $title ); - $user->title( Utils::trim_text( $title ) ); - } + $user->name( Utils::trim_text( $params{name} ) ) if $params{name}; + my $title = Utils::trim_text( $params{fms_extra_title} ); + $user->title( $title ) if $title; $update->user( $user ); # Just in case, make sure the user will have a name @@ -135,21 +123,16 @@ sub process_user : Private { return 1; } } - # Extract all the params to a hash to make them easier to work with - my %params = map { $_ => $c->get_param($_) } - ( 'rznvy', 'name', 'password_register', 'fms_extra_title' ); - - # cleanup the email address - my $email = $params{rznvy} ? lc $params{rznvy} : ''; - $email =~ s{\s+}{}g; - - $update->user( $c->model('DB::User')->find_or_new( { email => $email } ) ) + my $parsed = FixMyStreet::SMS->parse_username($params{username}); + my $type = $parsed->{type} || 'email'; + $type = 'email' unless FixMyStreet->config('SMS_AUTHENTICATION'); + $update->user( $c->model('DB::User')->find_or_new( { $type => $parsed->{username} } ) ) unless $update->user; - # The user is trying to sign in. We only care about email from the params. + # 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') ) { - unless ( $c->forward( '/auth/sign_in', [ $email ] ) ) { - $c->stash->{field_errors}->{password} = _('There was a problem with your email/password combination. If you cannot remember your password, or do not have one, please fill in the ‘sign in by email’ section of the form.'); + unless ( $c->forward( '/auth/sign_in', [ $params{username} ] ) ) { + $c->stash->{field_errors}->{password} = _('There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the ‘No’ section of the form.'); return 1; } my $user = $c->user->obj; @@ -328,8 +311,6 @@ sub process_update : Private { $update->extra( $extra ); } - $c->log->debug( 'name is ' . $c->get_param('name') ); - $c->stash->{add_alert} = $c->get_param('add_alert'); return 1; @@ -372,7 +353,7 @@ sub check_for_errors : Private { $c->stash->{is_social_user} = $c->get_param('facebook_sign_in') || $c->get_param('twitter_sign_in'); if ( $c->stash->{is_social_user} ) { delete $field_errors{name}; - delete $field_errors{email}; + delete $field_errors{username}; } if ( my $photo_error = delete $c->stash->{photo_error} ) { @@ -445,11 +426,7 @@ sub save_update : Private { } if ( $c->cobrand->never_confirm_updates ) { - if ( $update->user->in_storage() ) { - $update->user->update(); - } else { - $update->user->insert(); - } + $update->user->update_or_insert; $update->confirm(); } elsif ( $c->forward('/report/new/created_as_someone_else', [ $update->problem->bodies_str ]) ) { # If created on behalf of someone else, we automatically confirm it, @@ -464,7 +441,6 @@ sub save_update : Private { } elsif ( $c->user && $c->user->id == $update->user->id ) { # Logged in and same user, so can confirm update straight away - $c->log->debug( 'user exists' ); $update->user->update; $update->confirm; } else { @@ -473,12 +449,7 @@ sub save_update : Private { $update->user->discard_changes(); } - if ( $update->in_storage ) { - $update->update; - } - else { - $update->insert; - } + $update->update_or_insert; return 1; } @@ -507,28 +478,108 @@ sub redirect_or_confirm_creation : Private { return 1; } - # otherwise create a confirm token and email it to them. - my $data = $c->stash->{token_data} || {}; - my $token = $c->model("DB::Token")->create( - { - scope => 'comment', - data => { - %$data, - id => $update->id, - add_alert => ( $c->get_param('add_alert') ? 1 : 0 ), - } - } - ); + my $data = $c->stash->{token_data}; + $data->{id} = $update->id; + $data->{add_alert} = $c->get_param('add_alert') ? 1 : 0; + + if ($update->user->email_verified) { + $c->forward('send_confirmation_email'); + # tell user that they've been sent an email + $c->stash->{template} = 'email_sent.html'; + $c->stash->{email_type} = 'update'; + } elsif ($update->user->phone_verified) { + $c->forward('send_confirmation_text'); + } else { + warn "Reached update confirmation with no username verification"; + } + + return 1; +} + +sub send_confirmation_email : Private { + my ( $self, $c ) = @_; + + my $update = $c->stash->{update}; + my $token = $c->model("DB::Token")->create( { + scope => 'comment', + data => $c->stash->{token_data}, + } ); + my $template = 'update-confirm.txt'; $c->stash->{token_url} = $c->uri_for_email( '/C', $token->token ); - $c->send_email( 'update-confirm.txt', { - to => $update->name - ? [ [ $update->user->email, $update->name ] ] - : $update->user->email, + $c->send_email( $template, { + to => [ $update->name ? [ $update->user->email, $update->name ] : $update->user->email ], } ); +} + +sub send_confirmation_text : Private { + my ( $self, $c ) = @_; + my $update = $c->stash->{update}; + $c->forward('/auth/phone/send_token', [ $c->stash->{token_data}, 'comment', $update->user->phone ]); + $c->stash->{submit_url} = '/report/update/text'; +} + +sub confirm_by_text : Path('text') { + my ( $self, $c ) = @_; + + my $token = $c->stash->{token} = $c->get_param('token'); + my $code = $c->get_param('code') || ''; + + my $data = $c->stash->{token_data} = $c->forward('/auth/get_token', [ $token, 'comment' ]) || return; + if ($data->{code} ne $code) { + $c->stash->{template} = 'auth/smsform.html'; + $c->stash->{submit_url} = '/report/update/text'; + $c->stash->{incorrect_code} = 1; + return; + } + + $c->detach('process_confirmation'); +} + +sub process_confirmation : Private { + my ( $self, $c ) = @_; + + $c->stash->{template} = 'tokens/confirm_update.html'; + my $data = $c->stash->{token_data}; - # tell user that they've been sent an email - $c->stash->{template} = 'email_sent.html'; - $c->stash->{email_type} = 'update'; + unless ($c->stash->{update}) { + $c->stash->{update} = $c->model('DB::Comment')->find({ id => $data->{id} }) || return; + } + my $comment = $c->stash->{update}; + + # check that this email or domain are not the cause of abuse. If so hide it. + if ( $comment->is_from_abuser ) { + $c->stash->{template} = 'tokens/abuse.html'; + return; + } + + if ( $comment->state ne 'unconfirmed' ) { + my $report_uri = $c->cobrand->base_url_for_report( $comment->problem ) . $comment->problem->url; + $c->res->redirect($report_uri); + return; + } + + if ( $data->{name} || $data->{password} ) { + for (qw(name facebook_id twitter_id)) { + $comment->user->$_( $data->{$_} ) if $data->{$_}; + } + $comment->user->password( $data->{password}, 1 ) if $data->{password}; + $comment->user->update; + } + + if ($comment->user->email_verified) { + $c->authenticate( { email => $comment->user->email, email_verified => 1 }, 'no_password' ); + } elsif ($comment->user->phone_verified) { + $c->authenticate( { phone => $comment->user->phone, phone_verified => 1 }, 'no_password' ); + } else { + warn "Reached user authentication with no username verification"; + } + $c->set_session_cookie_expire(0); + + $c->stash->{update}->confirm; + $c->stash->{update}->update; + $c->forward('update_problem'); + $c->stash->{add_alert} = $data->{add_alert}; + $c->forward('signup_for_alerts'); return 1; } diff --git a/perllib/FixMyStreet/App/Controller/Tokens.pm b/perllib/FixMyStreet/App/Controller/Tokens.pm index 1d4438828..bb6140e0a 100644 --- a/perllib/FixMyStreet/App/Controller/Tokens.pm +++ b/perllib/FixMyStreet/App/Controller/Tokens.pm @@ -45,10 +45,10 @@ sub confirm_problem : Path('/P') { # Load the problem my $data = $auth_token->data; $data = { id => $data } unless ref $data; + $c->stash->{token_data} = $data; - my $problem_id = $data->{id}; # Look at all problems, not just cobrand, in case am approving something we don't actually show - my $problem = $c->model('DB::Problem')->find( { id => $problem_id } ) + my $problem = $c->model('DB::Problem')->find( { id => $data->{id} } ) || $c->detach('token_error'); $c->stash->{report} = $problem; @@ -56,64 +56,7 @@ sub confirm_problem : Path('/P') { if $problem->state eq 'unconfirmed' && $auth_token->created < DateTime->now->subtract( months => 1 ); - # check that this email or domain are not the cause of abuse. If so hide it. - if ( $problem->is_from_abuser ) { - $problem->update( - { state => 'hidden', lastupdate => \'current_timestamp' } ); - $c->stash->{template} = 'tokens/abuse.html'; - return; - } - - # For Zurich, email confirmation simply sets a flag, it does not change the - # problem state, log in, or anything else - if ($c->cobrand->moniker eq 'zurich') { - $problem->set_extra_metadata( email_confirmed => 1 ); - $problem->update( { - confirmed => \'current_timestamp', - } ); - - if ( $data->{name} || $data->{password} ) { - $problem->user->name( $data->{name} ) if $data->{name}; - $problem->user->phone( $data->{phone} ) if $data->{phone}; - $problem->user->update; - } - - return 1; - } - - if ($problem->state ne 'unconfirmed') { - my $report_uri = $c->cobrand->base_url_for_report( $problem ) . $problem->url; - $c->res->redirect($report_uri); - return; - } - - # We have an unconfirmed problem - $problem->update( - { - state => 'confirmed', - confirmed => \'current_timestamp', - lastupdate => \'current_timestamp', - } - ); - - # Subscribe problem reporter to email updates - $c->forward( '/report/new/create_reporter_alert' ); - - # log the problem creation user in to the site - if ( $data->{name} || $data->{password} ) { - $problem->user->name( $data->{name} ) if $data->{name}; - $problem->user->phone( $data->{phone} ) if $data->{phone}; - $problem->user->password( $data->{password}, 1 ) if $data->{password}; - $problem->user->title( $data->{title} ) if $data->{title}; - $problem->user->facebook_id( $data->{facebook_id} ) if $data->{facebook_id}; - $problem->user->twitter_id( $data->{twitter_id} ) if $data->{twitter_id}; - $problem->user->update; - } - $c->authenticate( { email => $problem->user->email, email_verified => 1 }, 'no_password' ); - $c->set_session_cookie_expire(0); - - $c->stash->{created_report} = 'fromemail'; - return 1; + $c->forward('/report/new/process_confirmation'); } =head2 redirect_to_partial_problem @@ -205,11 +148,9 @@ sub confirm_update : Path('/C') { $c->forward( 'load_auth_token', [ $token_code, 'comment' ] ); # Load the update - my $data = $auth_token->data; - my $comment_id = $data->{id}; - $c->stash->{add_alert} = $data->{add_alert}; + my $data = $c->stash->{token_data} = $auth_token->data; - my $comment = $c->model('DB::Comment')->find( { id => $comment_id } ) + my $comment = $c->model('DB::Comment')->find( { id => $data->{id} } ) || $c->detach('token_error'); $c->stash->{update} = $comment; @@ -217,32 +158,7 @@ sub confirm_update : Path('/C') { if $comment->state ne 'confirmed' && $auth_token->created < DateTime->now->subtract( months => 1 ); - # check that this email or domain are not the cause of abuse. If so hide it. - if ( $comment->is_from_abuser ) { - $c->stash->{template} = 'tokens/abuse.html'; - return; - } - - if ( $comment->state ne 'unconfirmed' ) { - my $report_uri = $c->cobrand->base_url_for_report( $comment->problem ) . $comment->problem->url; - $c->res->redirect($report_uri); - return; - } - - if ( $data->{name} || $data->{password} ) { - $comment->user->name( $data->{name} ) if $data->{name}; - $comment->user->password( $data->{password}, 1 ) if $data->{password}; - $comment->user->facebook_id( $data->{facebook_id} ) if $data->{facebook_id}; - $comment->user->twitter_id( $data->{twitter_id} ) if $data->{twitter_id}; - $comment->user->update; - } - - $c->authenticate( { email => $comment->user->email, email_verified => 1 }, 'no_password' ); - $c->set_session_cookie_expire(0); - - $c->forward('/report/update/confirm'); - - return 1; + $c->forward('/report/update/process_confirmation'); } sub load_questionnaire : Private { diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm index 0b6c6bc57..4da9ef83a 100644 --- a/perllib/FixMyStreet/DB/Result/User.pm +++ b/perllib/FixMyStreet/DB/Result/User.pm @@ -117,6 +117,7 @@ __PACKAGE__->load_components("+FixMyStreet::DB::RABXColumn"); __PACKAGE__->rabx_column('extra'); use Moo; +use FixMyStreet::SMS; use mySociety::EmailUtil; use namespace::clean -except => [ 'meta' ]; @@ -178,11 +179,19 @@ sub check_for_errors { $errors{name} = _('Please enter your name'); } - if ( $self->email !~ /\S/ ) { - $errors{email} = _('Please enter your email'); - } - elsif ( !mySociety::EmailUtil::is_valid_email( $self->email ) ) { - $errors{email} = _('Please enter a valid email'); + if ($self->email_verified) { + if ($self->email !~ /\S/) { + $errors{username} = _('Please enter your email'); + } elsif (!mySociety::EmailUtil::is_valid_email($self->email)) { + $errors{username} = _('Please enter a valid email'); + } + } elsif ($self->phone_verified) { + my $parsed = FixMyStreet::SMS->parse_username($self->phone); + if (!$parsed->{phone}) { + $errors{username} = _('Please check your phone number is correct'); + } elsif (!$parsed->{phone}->is_mobile) { + $errors{username} = _('Please enter a mobile number'); + } } return \%errors; diff --git a/perllib/FixMyStreet/Roles/Abuser.pm b/perllib/FixMyStreet/Roles/Abuser.pm index fc76565ca..e2e9eb19e 100644 --- a/perllib/FixMyStreet/Roles/Abuser.pm +++ b/perllib/FixMyStreet/Roles/Abuser.pm @@ -13,9 +13,9 @@ Returns true if the user's email or its domain is listed in the 'abuse' table. sub is_from_abuser { my $self = shift; - # get the domain my $email = $self->user->email; - my ($domain) = $email =~ m{ @ (.*) \z }x; + my ($domain) = $email =~ m{ @ (.*) \z }x if $email; + my $phone = $self->user->phone; # search for an entry in the abuse table my $abuse_rs = $self->result_source->schema->resultset('Abuse'); @@ -23,6 +23,7 @@ sub is_from_abuser { return $abuse_rs->find( { email => $email } ) || $abuse_rs->find( { email => $domain } ) + || $abuse_rs->find( { email => $phone } ) || undef; } diff --git a/perllib/FixMyStreet/TestMech.pm b/perllib/FixMyStreet/TestMech.pm index 20f7a3ace..dbfc94286 100644 --- a/perllib/FixMyStreet/TestMech.pm +++ b/perllib/FixMyStreet/TestMech.pm @@ -65,11 +65,12 @@ Create a test user (or find it and return if it already exists). sub create_user_ok { my $self = shift; - my ( $email, %extra ) = @_; + my ( $username, %extra ) = @_; - my $params = { email => $email, %extra }; + my $params = { %extra }; + $username =~ /@/ ? $params->{email} = $username : $params->{phone} = $username; my $user = FixMyStreet::DB->resultset('User')->find_or_create($params); - ok $user, "found/created user for $email"; + ok $user, "found/created user for $username"; return $user; } @@ -78,15 +79,15 @@ sub create_user_ok { $user = $mech->log_in_ok( $email_address ); -Log in with the email given. If email does not match an account then create one. +Log in with the email/phone given. If email/phone does not match an account then create one. =cut sub log_in_ok { my $mech = shift; - my $email = shift; + my $username = shift; - my $user = $mech->create_user_ok($email); + my $user = $mech->create_user_ok($username); # remember the old password and then change it to a known one my $old_password = $user->password || ''; @@ -95,7 +96,7 @@ sub log_in_ok { # log in $mech->get_ok('/auth'); $mech->submit_form_ok( - { with_fields => { username => $email, password_sign_in => 'secret' } }, + { with_fields => { username => $username, password_sign_in => 'secret' } }, "sign in using form" ); $mech->logged_in_ok; @@ -135,6 +136,7 @@ sub log_out_ok { $mech->delete_user( $user ); $mech->delete_user( $email ); + $mech->delete_user( $phone ); Delete the current user, including linked objects like problems etc. Can be either a user object or an email address. @@ -142,14 +144,14 @@ either a user object or an email address. =cut sub delete_user { - my $mech = shift; - my $email_or_user = shift; - - my $user = - ref $email_or_user - ? $email_or_user - : FixMyStreet::DB->resultset('User') - ->find( { email => $email_or_user } ); + my $mech = shift; + my $user_or_username = shift; + + my $user = ref $user_or_username ? $user_or_username : undef; + $user = FixMyStreet::DB->resultset('User')->find( { email => $user_or_username } ) + unless $user; + $user = FixMyStreet::DB->resultset('User')->find( { phone => $user_or_username } ) + unless $user; # If no user found we can't delete them return 1 unless $user; diff --git a/t/Mock/Twilio.pm b/t/Mock/Twilio.pm index b35166704..eaad30b76 100644 --- a/t/Mock/Twilio.pm +++ b/t/Mock/Twilio.pm @@ -7,6 +7,14 @@ has texts => ( default => sub { [] }, ); +sub get_text_code { + my $self = shift; + my $text = shift @{$self->texts}; + return unless $text; + my ($code) = $text->{Body} =~ /(\d+)/; + return $code; +} + sub dispatch_request { my $self = shift; diff --git a/t/app/controller/auth_phone.t b/t/app/controller/auth_phone.t index a2f8f9cac..8673f5c62 100644 --- a/t/app/controller/auth_phone.t +++ b/t/app/controller/auth_phone.t @@ -56,8 +56,7 @@ subtest 'Log in using mobile, by text' => sub { }, 'submit incorrect code'); $mech->content_contains('Try again'); - my $text = shift @{$twilio->texts}; - my ($code) = $text->{Body} =~ /(\d+)/; + my $code = $twilio->get_text_code; $mech->submit_form_ok({ with_fields => { code => $code } }, 'submit correct code'); diff --git a/t/app/controller/auth_social.t b/t/app/controller/auth_social.t index d16a0102e..031fb8d9e 100644 --- a/t/app/controller/auth_social.t +++ b/t/app/controller/auth_social.t @@ -102,13 +102,7 @@ for my $fb_state ( 'refused', 'no email', 'existing UID', 'okay' ) { $mech->content_contains('We need your email address, please give it below.'); # We don't have an email, so check that we can still submit it, # and the ID carries through the confirmation - if ($page eq 'update') { - $fields->{rznvy} = $fb_email; - } elsif ($page eq 'report') { - $fields->{email} = $fb_email; - } else { - $fields->{username} = $fb_email; - } + $fields->{username} = $fb_email; $fields->{name} = 'Ffion Tester'; $mech->submit_form(with_fields => $fields); $mech->content_contains('Nearly done! Now check your email'); @@ -216,13 +210,7 @@ for my $tw_state ( 'refused', 'existing UID', 'no email' ) { $mech->content_contains('We need your email address, please give it below.'); # We don't have an email, so check that we can still submit it, # and the ID carries through the confirmation - if ($page eq 'update') { - $fields->{rznvy} = $tw_email; - } elsif ($page eq 'report') { - $fields->{email} = $tw_email; - } else { - $fields->{username} = $tw_email; - } + $fields->{username} = $tw_email; $fields->{name} = 'Ffion Tester'; $mech->submit_form(with_fields => $fields); $mech->content_contains('Nearly done! Now check your email'); diff --git a/t/app/controller/report_as_other.t b/t/app/controller/report_as_other.t index daa213e8c..91644e8ce 100644 --- a/t/app/controller/report_as_other.t +++ b/t/app/controller/report_as_other.t @@ -47,7 +47,7 @@ subtest "Body user, has permission to add report as another user" => sub { detail => 'Test report details.', category => 'Potholes', name => 'Another User', - email => 'another@example.net', + username => 'another@example.net', ); is $report->name, 'Another User', 'report name is given name'; is $report->user->name, 'Another User', 'user name matches'; @@ -66,7 +66,7 @@ subtest "Body user, has permission to add report as another (existing) user" => detail => 'Test report details.', category => 'Potholes', name => 'Existing Yooser', - email => 'existing@example.net', + username => 'existing@example.net', ); is $report->name, 'Existing Yooser', 'report name is given name'; is $report->user->name, 'Existing User', 'user name remains same'; @@ -108,7 +108,7 @@ subtest "Body user, has permission to add update as another user" => sub { form_as => 'another_user', update => 'Test Update', name => 'Another User', - rznvy => 'another2@example.net', + username => 'another2@example.net', ); is $update->name, 'Another User', 'update name is given name'; is $update->user->name, 'Another User', 'user name matches'; @@ -124,7 +124,7 @@ subtest "Body user, has permission to add update as another (existing) user" => form_as => 'another_user', update => 'Test Update', name => 'Existing Yooser', - rznvy => 'existing@example.net', + username => 'existing@example.net', ); is $update->name, 'Existing Yooser', 'update name is given name'; is $update->user->name, 'Existing User', 'user name remains same'; diff --git a/t/app/controller/report_display.t b/t/app/controller/report_display.t index 4d73a5204..f0913fbd2 100644 --- a/t/app/controller/report_display.t +++ b/t/app/controller/report_display.t @@ -128,7 +128,7 @@ subtest "test a good report" => sub { my %fields = ( name => '', - rznvy => '', + username => '', update => '', add_alert => 1, # defaults to true fixed => undef diff --git a/t/app/controller/report_import.t b/t/app/controller/report_import.t index 47113198e..e4a202db7 100644 --- a/t/app/controller/report_import.t +++ b/t/app/controller/report_import.t @@ -362,14 +362,12 @@ subtest "Submit a correct entry (with location) to cobrand" => sub { photo2 => '', photo3 => '', phone => '', - email => 'test-ll@example.com', + username => 'test-ll@example.com', }, "check imported fields are shown" or diag Dumper( $mech->visible_form_values ); use Data::Dumper; - my $user = - FixMyStreet::App->model('DB::User') - ->find( { email => 'test-ll@example.com' } ); + my $user = FixMyStreet::App->model('DB::User')->find( { email => 'test-ll@example.com' } ); ok $user, "Found a user"; my $report = $user->problems->first; diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t index ab6b5d78e..efe392eab 100644 --- a/t/app/controller/report_new.t +++ b/t/app/controller/report_new.t @@ -101,6 +101,7 @@ foreach my $test ( photo3 => '', name => '', may_show_name => '1', + username => '', email => '', phone => '', password_sign_in => '', @@ -127,6 +128,7 @@ foreach my $test ( photo3 => '', name => '', may_show_name => '1', + username => '', email => '', phone => '', category => 'Something bad', @@ -156,6 +158,7 @@ foreach my $test ( photo3 => '', name => '', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -182,6 +185,7 @@ foreach my $test ( photo3 => '', name => '', may_show_name => undef, + username => '', email => '', phone => '', category => 'Street lighting', @@ -208,6 +212,7 @@ foreach my $test ( photo3 => '', name => 'Bob Jones', may_show_name => undef, + username => '', email => '', phone => '', category => 'Street lighting', @@ -233,6 +238,7 @@ foreach my $test ( photo3 => '', name => 'Bob Jones', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -258,6 +264,7 @@ foreach my $test ( photo3 => '', name => 'Bob Jones', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -283,6 +290,7 @@ foreach my $test ( photo3 => '', name => 'DUDE', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -307,6 +315,7 @@ foreach my $test ( photo3 => '', name => 'anonymous', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -331,14 +340,15 @@ foreach my $test ( photo3 => '', name => 'Joe Smith', may_show_name => '1', - email => 'not an email', + username => 'not an email', + email => '', phone => '', category => 'Street lighting', password_sign_in => '', password_register => '', remember_me => undef, }, - changes => { email => 'notanemail', }, + changes => { username => 'notanemail', email => 'notanemail' }, errors => [ 'Please enter a valid email', ], }, { @@ -352,6 +362,7 @@ foreach my $test ( photo3 => '', name => '', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -379,7 +390,8 @@ foreach my $test ( photo3 => '', name => ' Bob Jones ', may_show_name => '1', - email => ' BOB @ExAmplE.COM ', + username => ' BOB @ExAmplE.COM ', + email => '', phone => '', category => 'Street lighting', password_sign_in => '', @@ -388,6 +400,7 @@ foreach my $test ( }, changes => { name => 'Bob Jones', + username => 'bob@example.com', email => 'bob@example.com', }, errors => [ 'Please enter a subject', 'Please enter some details', ], @@ -403,6 +416,7 @@ foreach my $test ( photo3 => '', name => 'Bob Jones', may_show_name => '1', + username => 'bob@example.com', email => 'bob@example.com', phone => '', category => 'Street lighting', @@ -426,6 +440,7 @@ foreach my $test ( photo3 => '', name => 'Bob Jones', may_show_name => '1', + username => 'bob@example.com', email => 'bob@example.com', phone => '', category => 'Street lighting', @@ -449,6 +464,7 @@ foreach my $test ( photo3 => '', name => 'Bob Jones', may_show_name => '1', + username => 'bob@example.com', email => 'bob@example.com', phone => '', category => 'Street lighting', @@ -560,7 +576,7 @@ foreach my $test ( photo1 => '', name => 'Joe Bloggs', may_show_name => '1', - email => 'test-1@example.com', + username => 'test-1@example.com', phone => '07903 123 456', category => 'Street lighting', password_register => $test->{password} ? 'secret' : '', @@ -674,7 +690,7 @@ subtest "test password errors for a user who is signing in as they report" => su title => 'Test Report', detail => 'Test report details.', photo1 => '', - email => 'test-2@example.com', + username => 'test-2@example.com', password_sign_in => 'secret1', category => 'Street lighting', } @@ -685,7 +701,7 @@ subtest "test password errors for a user who is signing in as they report" => su # check that we got the errors expected is_deeply $mech->page_errors, [ - "There was a problem with your email/password combination. If you cannot remember your password, or do not have one, please fill in the \x{2018}sign in by email\x{2019} section of the form.", + "There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the \x{2018}No\x{2019} section of the form.", ], "check there were errors"; }; @@ -726,7 +742,7 @@ subtest "test report creation for a user who is signing in as they report" => su title => 'Test Report', detail => 'Test report details.', photo1 => '', - email => 'test-2@example.com', + username => 'test-2@example.com', password_sign_in => 'secret2', category => 'Street lighting', } @@ -947,7 +963,7 @@ subtest "test report creation for a category that is non public" => sub { title => 'Test Report', detail => 'Test report details.', photo1 => '', - email => 'test-2@example.com', + username => 'test-2@example.com', name => 'Joe Bloggs', category => 'Street lighting', } @@ -1135,7 +1151,7 @@ for my $test ( title => "Test Report", detail => 'Test report details.', photo1 => '', - email => 'firstlast@example.com', + username => 'firstlast@example.com', may_show_name => '1', phone => '07903 123 456', category => 'Trees', @@ -1167,9 +1183,7 @@ for my $test ( # confirm token in order to update the user details $mech->get_ok($url); - my $user = - FixMyStreet::App->model('DB::User') - ->find( { email => 'firstlast@example.com' } ); + my $user = FixMyStreet::App->model('DB::User')->find( { email => 'firstlast@example.com' } ); my $report = $user->problems->first; ok $report, "Found the report"; @@ -1284,7 +1298,7 @@ subtest "test Hart" => sub { $mech->submit_form_ok( { with_fields => { pc => 'GU51 4AE' } }, "submit location" ); $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" ); my %optional_fields = $test->{confirm} ? () : - ( email => $test_email, phone => '07903 123 456' ); + ( username => $test_email, phone => '07903 123 456' ); # we do this as otherwise test::www::mechanize::catalyst # goes to the value set in ->host above irregardless and @@ -1424,7 +1438,7 @@ subtest "unresponsive body handling works" => sub { detail => 'Test report details.', photo1 => '', name => 'Joe Bloggs', - email => $test_email, + username => $test_email, may_show_name => '1', phone => '07903 123 456', category => 'Trees', @@ -1497,7 +1511,7 @@ subtest "unresponsive body handling works" => sub { detail => 'Test report details.', photo1 => '', name => 'Joe Bloggs', - email => $test_email, + username => $test_email, may_show_name => '1', phone => '07903 123 456', category => 'Trees', @@ -1618,7 +1632,7 @@ subtest "extra google analytics code displayed on email confirmation problem cre title => "Test Report", detail => 'Test report details.', photo1 => '', - email => 'firstlast@example.com', + username => 'firstlast@example.com', name => 'Test User', may_show_name => '1', phone => '07903 123 456', @@ -1639,9 +1653,7 @@ subtest "extra google analytics code displayed on email confirmation problem cre $mech->get_ok($url); # find the report - my $user = - FixMyStreet::App->model('DB::User') - ->find( { email => 'firstlast@example.com' } ); + my $user = FixMyStreet::App->model('DB::User')->find( { email => 'firstlast@example.com' } ); my $report = $user->problems->first; ok $report, "Found the report"; diff --git a/t/app/controller/report_new_open311.t b/t/app/controller/report_new_open311.t index 9a4a81182..0224e7e47 100644 --- a/t/app/controller/report_new_open311.t +++ b/t/app/controller/report_new_open311.t @@ -54,6 +54,7 @@ foreach my $test ( photo3 => '', name => '', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -76,7 +77,7 @@ foreach my $test ( title => 'test', detail => 'test detail', name => 'Test User', - email => 'testopen311@example.com', + username => 'testopen311@example.com', category => 'Street lighting', number => 27, }, @@ -100,7 +101,7 @@ foreach my $test ( $mech->clear_emails_ok; # check that the user does not exist - my $test_email = $test->{submit_with}->{email}; + my $test_email = $test->{submit_with}->{username}; my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } ); if ( $user ) { $user->problems->delete; diff --git a/t/app/controller/report_new_text.t b/t/app/controller/report_new_text.t new file mode 100644 index 000000000..94f350eba --- /dev/null +++ b/t/app/controller/report_new_text.t @@ -0,0 +1,371 @@ +use FixMyStreet::TestMech; +use t::Mock::Twilio; + +my $twilio = t::Mock::Twilio->new; +LWP::Protocol::PSGI->register($twilio->to_psgi_app, host => 'api.twilio.com'); + +# disable info logs for this test run +FixMyStreet::App->log->disable('info'); +END { FixMyStreet::App->log->enable('info'); } + +my $mech = FixMyStreet::TestMech->new; + +my $body = $mech->create_body_ok(2651, 'City of Edinburgh Council'); +$mech->create_contact_ok( body_id => $body->id, category => 'Street lighting', email => 'highways@example.com' ); +$mech->create_contact_ok( body_id => $body->id, category => 'Trees', email => 'trees@example.com' ); + +# test that phone number validation works okay +foreach my $test ( + { + msg => 'invalid number', + pc => 'EH1 1BB', + fields => { + username => '0121 4960000000', email => '', phone => '', + title => 'Title', detail => 'Detail', name => 'Bob Jones', + category => 'Street lighting', + may_show_name => '1', remember_me => undef, + photo1 => '', photo2 => '', photo3 => '', + password_register => '', password_sign_in => '', + }, + changes => { + username => '01214960000000', + phone => '01214960000000', + }, + errors => [ 'Please check your phone number is correct' ], + }, + { + msg => 'landline number', + pc => 'EH1 1BB', + fields => { + username => '0121 4960000', email => '', phone => '', + title => 'Title', detail => 'Detail', name => 'Bob Jones', + category => 'Street lighting', + may_show_name => '1', remember_me => undef, + photo1 => '', photo2 => '', photo3 => '', + password_register => '', password_sign_in => '', + }, + changes => { + username => '+44 121 496 0000', + phone => '+44 121 496 0000', + }, + errors => [ 'Please enter a mobile number', ], + }, + ) +{ + subtest "check form errors where $test->{msg}" => sub { + $mech->get_ok('/around'); + + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + SMS_AUTHENTICATION => 1, + PHONE_COUNTRY => 'GB', + }, sub { + $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } }, + "submit location" ); + is_deeply $mech->page_errors, [], "no errors for pc '$test->{pc}'"; + + # click through to the report page + $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, + "follow 'skip this step' link" ); + + # submit the main form + $mech->submit_form_ok( { with_fields => $test->{fields} }, "submit form" ); + }; + + # check that we got the errors expected + is_deeply [ sort @{$mech->page_errors} ], [ sort @{$test->{errors}} ], "check errors"; + + # check that fields have changed as expected + my $new_values = { + %{ $test->{fields} }, # values added to form + %{ $test->{changes} }, # changes we expect + }; + is_deeply $mech->visible_form_values, $new_values, + "values correctly changed"; + }; +} + +my $test_phone = '+61491570156'; +my $first_user; +foreach my $test ( + { + desc => 'does not have an account, does not set a password', + user => 0, password => 0, + }, + { + desc => 'does not have an account, sets a password', + user => 0, password => 1, + }, + { + desc => 'does have an account and is not signed in; does not sign in, does not set a password', + user => 1, password => 0, + }, + { + desc => 'does have an account and is not signed in; does not sign in, sets a password', + user => 1, password => 1, + }, +) { + subtest "test report creation for a user who " . $test->{desc} => sub { + $mech->log_out_ok; + + if ($test->{user}) { + my $user = FixMyStreet::App->model('DB::User')->find( { phone => $test_phone } ); + ok $user, "test user does exist"; + $user->problems->delete; + $user->name( 'Old Name' ); + $user->password( 'old_password' ); + $user->update; + } elsif (!$first_user) { + ok !FixMyStreet::App->model('DB::User')->find( { phone => $test_phone } ), + "test user does not exist"; + $first_user = 1; + } else { + # Not first pass, so will exist, but want no user to start, so delete it. + $mech->delete_user($test_phone); + } + + $mech->get_ok('/around'); + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + SMS_AUTHENTICATION => 1, + PHONE_COUNTRY => 'GB', + TWILIO_ACCOUNT_SID => 'AC123', + }, sub { + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB', } }, "submit location" ); + $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" ); + $mech->submit_form_ok( + { + button => 'submit_register', + with_fields => { + title => 'Test Report', detail => 'Test report details.', + photo1 => '', + name => 'Joe Bloggs', may_show_name => '1', + username => $test_phone, + category => 'Street lighting', + password_register => $test->{password} ? 'secret' : '', + } + }, + "submit good details" + ); + }; + + is_deeply $mech->page_errors, [], "check there were no errors"; + + my $user = FixMyStreet::App->model('DB::User')->find( { phone => $test_phone } ); + ok $user, "user found"; + if ($test->{user}) { + is $user->name, 'Old Name', 'name unchanged'; + ok $user->check_password('old_password'), 'password unchanged'; + } else { + is $user->name, undef, 'name not yet set'; + is $user->password, '', 'password not yet set for new user'; + } + + my $report = $user->problems->first; + ok $report, "Found the report"; + is $report->state, 'unconfirmed', "report not confirmed"; + is $report->bodies_str, $body->id; + + $mech->submit_form_ok({ with_fields => { code => '00000' } }); + $mech->content_contains('Try again'); + + my $code = $twilio->get_text_code; + $mech->submit_form_ok({ with_fields => { code => $code } }); + + $report->discard_changes; + is $report->state, 'confirmed', "Report is now confirmed"; + + $mech->get_ok( '/report/' . $report->id ); + + is $report->name, 'Joe Bloggs', 'name updated correctly'; + if ($test->{password}) { + ok $report->user->check_password('secret'), 'password updated correctly'; + } elsif ($test->{user}) { + ok $report->user->check_password('old_password'), 'password unchanged, as no new one given'; + } else { + is $report->user->password, '', 'password still not set, as none given'; + } + + # check that the reporter has an alert + my $alert = FixMyStreet::App->model('DB::Alert')->find( { + user => $report->user, + alert_type => 'new_updates', + parameter => $report->id, + } ); + ok $alert, "created new alert"; + + # user is created and logged in + $mech->logged_in_ok; + + # cleanup + $mech->delete_user($user) + if $test->{user} && $test->{password}; + }; +} + +# this test to make sure that we don't see spurious error messages about +# the name being blank when there is a sign in error +subtest "test password errors for a user who is signing in as they report" => sub { + $mech->log_out_ok; + + my $user = $mech->create_user_ok($test_phone); + ok $user->update( { + name => 'Joe Bloggs', + email => 'joe@example.net', + password => 'secret2', + } ), "set user details"; + + $mech->get_ok('/around'); + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + SMS_AUTHENTICATION => 1, + }, sub { + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB', } }, "submit location" ); + $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" ); + $mech->submit_form_ok( + { + button => 'submit_sign_in', + with_fields => { + title => 'Test Report', + detail => 'Test report details.', + photo1 => '', + username => $test_phone, + password_sign_in => 'secret1', + category => 'Street lighting', + } + }, + "submit with wrong password" + ); + }; + + # check that we got the errors expected + is_deeply $mech->page_errors, [ + "There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the \x{2018}No\x{2019} section of the form.", + ], "check there were errors"; +}; + +subtest "test report creation for a user who is signing in as they report" => sub { + $mech->log_out_ok; + $mech->cookie_jar({}); + + my $user = $mech->create_user_ok($test_phone); + + $mech->get_ok('/around'); + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + SMS_AUTHENTICATION => 1, + }, sub { + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB', } }, "submit location" ); + $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" ); + $mech->submit_form_ok( + { + button => 'submit_sign_in', + with_fields => { + title => 'Test Report', + detail => 'Test report details.', + photo1 => '', + username => $test_phone, + password_sign_in => 'secret2', + category => 'Street lighting', + } + }, + "submit good details" + ); + + # check that we got the message expected + $mech->content_contains( 'You have successfully signed in; please check and confirm your details are accurate:' ); + + # Now submit with a name + $mech->submit_form_ok( + { + with_fields => { + name => 'Joe Bloggs', + } + }, + "submit good details" + ); + }; + + my $report = $user->problems->first; + ok $report, "Found the report"; + $mech->content_contains('Thank you for reporting this issue'); + is $report->bodies_str, $body->id; + is $report->state, 'confirmed', "report is now confirmed"; + $mech->get_ok( '/report/' . $report->id ); + my $alert = FixMyStreet::App->model('DB::Alert')->find( { + user => $report->user, + alert_type => 'new_updates', + parameter => $report->id, + } ); + ok $alert, "created new alert"; + + $mech->logged_in_ok; +}; + +subtest "test report creation for a user who is logged in" => sub { + my $user = $mech->create_user_ok($test_phone); + $mech->get_ok('/around'); + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + }, sub { + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB', } }, "submit location" ); + $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" ); + is_deeply( + $mech->visible_form_values, + { + title => '', + detail => '', + may_show_name => '1', + name => 'Joe Bloggs', + email => 'joe@example.net', + photo1 => '', + photo2 => '', + photo3 => '', + category => '-- Pick a category --', + }, + "user's details prefilled" + ); + + $mech->submit_form_ok( + { + with_fields => { + title => "Test Report at cafĂ©", + detail => 'Test report details.', + photo1 => '', + name => 'Joe Bloggs', + may_show_name => '1', + category => 'Street lighting', + } + }, + "submit good details" + ); + }; + + my $report = $user->problems->first; + ok $report, "Found the report"; + is $report->bodies_str, $body->id; + $mech->content_contains('Thank you for reporting this issue'); + is $report->state, 'confirmed', "report is now confirmed"; + $mech->get_ok( '/report/' . $report->id ); + my $alert = FixMyStreet::App->model('DB::Alert')->find( { + user => $report->user, + alert_type => 'new_updates', + parameter => $report->id, + } ); + ok $alert, "created new alert"; + + $mech->logged_in_ok; + + $mech->get_ok( + '/ajax?bbox=' . ($report->longitude - 0.01) . ',' . ($report->latitude - 0.01) + . ',' . ($report->longitude + 0.01) . ',' . ($report->latitude + 0.01) + ); + $mech->content_contains( "Test Report at caf\xc3\xa9" ); +}; + +done_testing(); diff --git a/t/app/controller/report_update_text.t b/t/app/controller/report_update_text.t new file mode 100644 index 000000000..45b4e78c2 --- /dev/null +++ b/t/app/controller/report_update_text.t @@ -0,0 +1,307 @@ +use FixMyStreet::TestMech; +use t::Mock::Twilio; + +my $twilio = t::Mock::Twilio->new; +LWP::Protocol::PSGI->register($twilio->to_psgi_app, host => 'api.twilio.com'); + +my $mech = FixMyStreet::TestMech->new; +my $user = $mech->create_user_ok('test@example.com', name => 'Test User'); +my $user2 = $mech->create_user_ok('commenter@example.com', name => 'Commenter'); +my $body = $mech->create_body_ok(2504, 'Westminster City Council'); + +my $dt = DateTime->new( + year => 2011, + month => 04, + day => 16, + hour => 15, + minute => 47, + second => 23 +); + +my $report = FixMyStreet::App->model('DB::Problem')->find_or_create( + { + postcode => 'SW1A 1AA', + bodies_str => $body->id, + areas => ',105255,11806,11828,2247,2504,', + category => 'Other', + title => 'Test 2', + detail => 'Test 2 Detail', + used_map => 't', + name => 'Test User', + anonymous => 'f', + state => 'confirmed', + confirmed => $dt->ymd . ' ' . $dt->hms, + lang => 'en-gb', + service => '', + cobrand => 'default', + cobrand_data => '', + send_questionnaire => 't', + latitude => '51.5016605453401', + longitude => '-0.142497580865087', + user_id => $user->id, + } +); +my $report_id = $report->id; +ok $report, "created test report - $report_id"; + +my $comment = FixMyStreet::App->model('DB::Comment')->find_or_create( { + problem_id => $report_id, + user_id => $user2->id, + name => 'Other User', + mark_fixed => 'false', + text => 'This is some update text', + state => 'confirmed', + confirmed => $dt->ymd . ' ' . $dt->hms, + anonymous => 'f', +}); + +my $comment_id = $comment->id; +ok $comment, "created test update - $comment_id"; + +for my $test ( + { + desc => 'Invalid phone', + fields => { + username => '01214960000000', + update => 'Update', + name => 'Name', + photo1 => '', + photo2 => '', + photo3 => '', + fixed => undef, + add_alert => 1, + may_show_name => undef, + remember_me => undef, + password_sign_in => '', + password_register => '', + }, + changes => {}, + field_errors => [ 'Please check your phone number is correct' ] + }, + { + desc => 'landline number', + fields => { + username => '01214960000', + update => 'Update', + name => 'Name', + photo1 => '', + photo2 => '', + photo3 => '', + fixed => undef, + add_alert => 1, + may_show_name => undef, + remember_me => undef, + password_register => '', + password_sign_in => '', + }, + changes => { + username => '+44 121 496 0000', + }, + field_errors => [ 'Please enter a mobile number' ] + }, + ) +{ + subtest "submit an update - $test->{desc}" => sub { + $mech->get_ok("/report/$report_id"); + + FixMyStreet::override_config { + SMS_AUTHENTICATION => 1, + PHONE_COUNTRY => 'GB', + }, sub { + $mech->submit_form_ok( { with_fields => $test->{fields} }, 'submit update' ); + }; + + is_deeply $mech->page_errors, $test->{field_errors}, 'field errors'; + + my $values = { + %{ $test->{fields} }, + %{ $test->{changes} }, + }; + + is_deeply $mech->visible_form_values('updateForm'), $values, 'form changes'; + }; +} + +my $test_phone = '+61491570156'; +for my $test ( + { + desc => 'submit an update, unregistered, logged out', + form_values => { + submit_update => 1, + username => $test_phone, + update => 'Update from an unregistered user', + add_alert => undef, + name => 'Unreg User', + may_show_name => undef, + }, + }, + { + desc => 'submit an update, unregistered, logged out, sign up for alerts', + form_values => { + submit_update => 1, + username => $test_phone, + update => 'Update from an unregistered user', + add_alert => 1, + name => 'Unreg User', + may_show_name => undef, + }, + }, + { + desc => 'submit an update, registered, logged out, confirming by text', + registered => 1, + form_values => { + submit_update => 1, + username => $test_phone, + update => 'Update from a registered user', + add_alert => undef, + name => 'Reg User', + password_register => 'new_secret', + }, + }, +) { + subtest $test->{desc} => sub { + $mech->log_out_ok(); + my $user; + if ($test->{registered}) { + $user = $mech->create_user_ok( $test_phone ); + $user->update( { name => 'Mr Reg', password => 'secret2' } ); + } + + $mech->get_ok("/report/$report_id"); + FixMyStreet::override_config { + SMS_AUTHENTICATION => 1, + TWILIO_ACCOUNT_SID => 'AC123', + }, sub { + $mech->submit_form_ok( { with_fields => $test->{form_values} }, 'submit update'); + }; + $mech->content_contains('Nearly done! Now check your phone'); + + if ($user) { + $user->discard_changes; + ok $user->check_password( 'secret2' ), 'password unchanged'; + is $user->name, 'Mr Reg', 'name unchanged'; + } + + my ($token) = $mech->content =~ /name="token" value="([^"]*)"/; + $token = FixMyStreet::App->model('DB::Token')->find({ + token => $token, + scope => 'comment' + }); + ok $token, 'Token found in database'; + + my $update_id = $token->data->{id}; + my $add_alerts = $token->data->{add_alert}; + my $update = FixMyStreet::App->model('DB::Comment')->find( { id => $update_id } ); + + ok $update, 'found update in database'; + is $update->state, 'unconfirmed', 'update unconfirmed'; + my $details = $test->{form_values}; + is $update->user->phone, $details->{username}, 'update phone'; + is $update->user->phone_verified, 1; + is $update->text, $details->{update}, 'update text'; + is $add_alerts, $details->{add_alert} ? 1 : 0, 'do not sign up for alerts'; + + my $code = $twilio->get_text_code; + $mech->submit_form_ok( { with_fields => { code => '00000' } }); + $mech->content_contains('Try again'); + $mech->submit_form_ok( { with_fields => { code => $code } }); + + $mech->content_contains("/report/$report_id#update_$update_id"); + + if ($user) { + $user->discard_changes; + ok $user->check_password( 'new_secret' ), 'password changed'; + is $user->name, 'Reg User', 'name changed'; + } else { + $user = FixMyStreet::App->model( 'DB::User' )->find( { phone => $details->{username} } ); + ok $user, 'found user'; + } + + my $alert = FixMyStreet::App->model( 'DB::Alert' )->find( + { user => $user, alert_type => 'new_updates', confirmed => 1, } + ); + + ok $details->{add_alert} ? defined( $alert ) : !defined( $alert ), 'sign up for alerts'; + + $update->discard_changes; + is $update->state, 'confirmed', 'update confirmed'; + $mech->delete_user( $user ); + }; +} + +for my $test ( + { + desc => 'submit an update for a registered user, signing in with wrong password', + form_values => { + submit_update => 1, + username => $test_phone, + update => 'Update from a user', + add_alert => undef, + password_sign_in => 'secret', + }, + field_errors => [ + "There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the \x{2018}No\x{2019} section of the form.", + 'Please enter your name', # FIXME Not really necessary error + ], + }, + { + desc => 'submit an update for a registered user and sign in', + form_values => { + submit_update => 1, + username => $test_phone, + update => 'Update from a user', + add_alert => undef, + password_sign_in => 'secret2', + }, + message => 'You have successfully signed in; please check and confirm your details are accurate:', + } +) { + subtest $test->{desc} => sub { + # Set things up + my $user = $mech->create_user_ok( $test->{form_values}->{username} ); + my $pw = 'secret2'; + $user->update( { name => 'Mr Reg', password => $pw } ); + $report->comments->delete; + + $mech->log_out_ok(); + $mech->clear_emails_ok(); + $mech->get_ok("/report/$report_id"); + FixMyStreet::override_config { + SMS_AUTHENTICATION => 1, + }, sub { + $mech->submit_form_ok( + { + button => 'submit_sign_in', + with_fields => $test->{form_values} + }, + 'submit update' + ); + }; + + $mech->content_contains($test->{message}) if $test->{message}; + + is_deeply $mech->page_errors, $test->{field_errors}, 'check there were errors' + if $test->{field_errors}; + + SKIP: { + skip( "Incorrect password", 4 ) unless $test->{form_values}{password_sign_in} eq $pw; + + # Now submit with a name + $mech->submit_form_ok( + { with_fields => { name => 'Joe Bloggs', } }, + "submit good details" + ); + + $mech->content_contains('Thank you for updating this issue'); + + my $update = $report->comments->first; + ok $update, 'found update'; + is $update->text, $test->{form_values}->{update}, 'update text'; + is $update->user->phone, $test->{form_values}->{username}, 'update user'; + is $update->state, 'confirmed', 'update confirmed'; + $mech->delete_user( $update->user ); + } + }; +} + +done_testing(); diff --git a/t/app/controller/report_updates.t b/t/app/controller/report_updates.t index 0526b2fd7..7cb547081 100644 --- a/t/app/controller/report_updates.t +++ b/t/app/controller/report_updates.t @@ -196,7 +196,7 @@ for my $test ( { desc => 'No email, no message', fields => { - rznvy => '', + username => '', update => '', name => '', photo1 => '', @@ -215,7 +215,7 @@ for my $test ( { desc => 'Invalid email, no message', fields => { - rznvy => 'test', + username => 'test', update => '', name => '', photo1 => '', @@ -234,7 +234,7 @@ for my $test ( { desc => 'email with spaces, no message', fields => { - rznvy => 'test @ example. com', + username => 'test @ example. com', update => '', name => '', photo1 => '', @@ -248,14 +248,14 @@ for my $test ( password_sign_in => '', }, changes => { - rznvy => 'test@example.com', + username => 'test@example.com', }, field_errors => [ 'Please enter a message', 'Please enter your name' ] }, { desc => 'email with uppercase, no message', fields => { - rznvy => 'test@EXAMPLE.COM', + username => 'test@EXAMPLE.COM', update => '', name => '', photo1 => '', @@ -269,7 +269,7 @@ for my $test ( password_sign_in => '', }, changes => { - rznvy => 'test@example.com', + username => 'test@example.com', }, field_errors => [ 'Please enter a message', 'Please enter your name' ] }, @@ -297,7 +297,7 @@ for my $test ( desc => 'submit an update for a non registered user', initial_values => { name => '', - rznvy => '', + username => '', may_show_name => 1, add_alert => 1, photo1 => '', @@ -311,7 +311,7 @@ for my $test ( }, form_values => { submit_update => 1, - rznvy => 'unregistered@example.com', + username => 'unregistered@example.com', update => 'Update from an unregistered user', add_alert => undef, name => 'Unreg User', @@ -323,7 +323,7 @@ for my $test ( desc => 'submit an update for a non registered user and sign up', initial_values => { name => '', - rznvy => '', + username => '', may_show_name => 1, add_alert => 1, photo1 => '', @@ -337,7 +337,7 @@ for my $test ( }, form_values => { submit_update => 1, - rznvy => 'unregistered@example.com', + username => 'unregistered@example.com', update => "update from an\r\n\r\nunregistered user", add_alert => 1, name => 'Unreg User', @@ -395,14 +395,14 @@ for my $test ( ok $update, 'found update in database'; is $update->state, 'unconfirmed', 'update unconfirmed'; - is $update->user->email, $details->{rznvy}, 'update email'; + is $update->user->email, $details->{username}, 'update email'; is $update->text, $details->{update}, 'update text'; is $add_alerts, $details->{add_alert} ? 1 : 0, 'do not sign up for alerts'; $mech->get_ok( $url ); $mech->content_contains("/report/$report_id#update_$update_id"); - my $unreg_user = FixMyStreet::App->model( 'DB::User' )->find( { email => $details->{rznvy} } ); + my $unreg_user = FixMyStreet::App->model( 'DB::User' )->find( { email => $details->{username} } ); ok $unreg_user, 'found user'; @@ -427,7 +427,7 @@ for my $test ( desc => 'overriding email confirmation allows report confirmation with no email sent', initial_values => { name => '', - rznvy => '', + username => '', may_show_name => 1, add_alert => 1, photo1 => '', @@ -441,7 +441,7 @@ for my $test ( }, form_values => { submit_update => 1, - rznvy => 'unregistered@example.com', + username => 'unregistered@example.com', update => "update no email confirm", add_alert => 1, name => 'Unreg User', @@ -493,10 +493,10 @@ for my $test ( ok $update, 'found update in database'; is $update->state, 'confirmed', 'update confirmed'; - is $update->user->email, $details->{rznvy}, 'update email'; + is $update->user->email, $details->{username}, 'update email'; is $update->text, $details->{update}, 'update text'; - my $unreg_user = FixMyStreet::App->model( 'DB::User' )->find( { email => $details->{rznvy} } ); + my $unreg_user = FixMyStreet::App->model( 'DB::User' )->find( { email => $details->{username} } ); ok $unreg_user, 'found user'; @@ -972,13 +972,13 @@ for my $test ( desc => 'submit an update for a registered user, signing in with wrong password', form_values => { submit_update => 1, - rznvy => 'registered@example.com', + username => 'registered@example.com', update => 'Update from a user', add_alert => undef, password_sign_in => 'secret', }, field_errors => [ - "There was a problem with your email/password combination. If you cannot remember your password, or do not have one, please fill in the \x{2018}sign in by email\x{2019} section of the form.", + "There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the \x{2018}No\x{2019} section of the form.", 'Please enter your name', # FIXME Not really necessary error ], }, @@ -986,7 +986,7 @@ for my $test ( desc => 'submit an update for a registered user and sign in', form_values => { submit_update => 1, - rznvy => 'registered@example.com', + username => 'registered@example.com', update => 'Update from a user', add_alert => undef, password_sign_in => 'secret2', @@ -996,7 +996,7 @@ for my $test ( ) { subtest $test->{desc} => sub { # Set things up - my $user = $mech->create_user_ok( $test->{form_values}->{rznvy} ); + my $user = $mech->create_user_ok( $test->{form_values}->{username} ); my $pw = 'secret2'; $user->update( { name => 'Mr Reg', password => $pw } ); $report->comments->delete; @@ -1036,7 +1036,7 @@ for my $test ( my $update = $report->comments->first; ok $update, 'found update'; is $update->text, $test->{form_values}->{update}, 'update text'; - is $update->user->email, $test->{form_values}->{rznvy}, 'update user'; + is $update->user->email, $test->{form_values}->{username}, 'update user'; is $update->state, 'confirmed', 'update confirmed'; $mech->delete_user( $update->user ); } @@ -1053,7 +1053,7 @@ subtest 'submit an update for a registered user, creating update by email' => su $mech->submit_form_ok( { with_fields => { submit_update => 1, - rznvy => 'registered@example.com', + username => 'registered@example.com', update => 'Update from a user', add_alert => undef, name => 'New Name', @@ -1502,7 +1502,7 @@ for my $test ( fields => { submit_update => 1, name => 'Test User', - rznvy => 'test@example.com', + username => 'test@example.com', may_show_name => 1, update => 'update from owner', add_alert => undef, @@ -1524,7 +1524,7 @@ for my $test ( submit_update => 1, name => 'Test User', may_show_name => 1, - rznvy => 'test@example.com', + username => 'test@example.com', update => 'update from owner', add_alert => undef, fixed => 1, @@ -1589,7 +1589,7 @@ for my $test ( my $update = $report->comments->first; ok $update, 'found update'; is $update->text, $results->{update}, 'update text'; - is $update->user->email, $test->{fields}->{rznvy}, 'update user'; + is $update->user->email, $test->{fields}->{username}, 'update user'; is $update->state, 'unconfirmed', 'update confirmed'; is $update->anonymous, $test->{anonymous}, 'user anonymous'; diff --git a/t/cobrand/bromley.t b/t/cobrand/bromley.t index f3053c29a..a64337085 100644 --- a/t/cobrand/bromley.t +++ b/t/cobrand/bromley.t @@ -74,7 +74,7 @@ for my $test ( cobrand => 'bromley', fields => { submit_update => 1, - rznvy => 'unregistered@example.com', + username => 'unregistered@example.com', update => 'Update from an unregistered user', add_alert => undef, first_name => 'Unreg', @@ -87,7 +87,7 @@ for my $test ( cobrand => 'fixmystreet', fields => { submit_update => 1, - rznvy => 'unregistered@example.com', + username => 'unregistered@example.com', update => 'Update from an unregistered user', add_alert => undef, name => 'Unreg User', diff --git a/t/cobrand/form_extras.t b/t/cobrand/form_extras.t index f450d908e..84ded5bc1 100644 --- a/t/cobrand/form_extras.t +++ b/t/cobrand/form_extras.t @@ -37,7 +37,7 @@ FixMyStreet::override_config { detail => 'Test report details.', name => 'Joe Bloggs', may_show_name => '1', - email => 'test-1@example.com', + username => 'test-1@example.com', passport => '123456', password_register => '', } diff --git a/t/cobrand/oxfordshire.t b/t/cobrand/oxfordshire.t index a79a8f2a4..abafa1fe8 100644 --- a/t/cobrand/oxfordshire.t +++ b/t/cobrand/oxfordshire.t @@ -217,7 +217,7 @@ subtest 'response times messages displayed' => sub { title => 'Test Report', detail => 'Test report details.', photo1 => '', - email => 'test-2@example.com', + username => 'test-2@example.com', name => 'Test User', category => 'Pothole', } diff --git a/t/cobrand/zurich.t b/t/cobrand/zurich.t index 03b20b087..e0671db2a 100644 --- a/t/cobrand/zurich.t +++ b/t/cobrand/zurich.t @@ -808,7 +808,7 @@ subtest "photo must be supplied for categories that require it" => sub { $mech->get_ok('/report/new?lat=47.381817&lon=8.529156'); $mech->submit_form_ok({ with_fields => { detail => 'Problem-Bericht', - email => 'user@example.org', + username => 'user@example.org', category => 'Graffiti - photo required', }}); is $mech->res->code, 200, "missing photo shouldn't return anything but 200"; diff --git a/templates/web/base/report/new/form_user_loggedin.html b/templates/web/base/report/new/form_user_loggedin.html index e841845bf..bd4ce1cf7 100644 --- a/templates/web/base/report/new/form_user_loggedin.html +++ b/templates/web/base/report/new/form_user_loggedin.html @@ -29,12 +29,19 @@ </select> [% END %] - <label for="form_email">[% loc('Email address') %]</label> - <input class="form-control" id="form_email" name="email" +[% IF c.user.phone_verified %] + <label for="form_phone">[% loc('Phone number') %]</label> + <input class="form-control" id="form_phone" name="phone" disabled type="text" value="[% c.user.phone | html %]"> +[% END %] + +[% IF c.user.email_verified %] + <label for="form_username">[% loc('Email address') %]</label> + <input class="form-control" id="form_username" name="username" [%- IF NOT can_contribute_as_another_user -%] disabled [%- END -%] type="text" value="[% c.user.email | html %]"> +[% END %] [% INCLUDE 'report/new/extra_name.html' %] [% PROCESS 'user/_anonymity.html' anonymous = report.anonymous %] @@ -56,8 +63,14 @@ <label class="inline" for="form_may_show_name">[% loc('Show my name publicly') %] </label> </div> +[% IF NOT c.user.phone_verified %] <label for="form_phone">[% loc('Phone number (optional)') %]</label> <input class="form-control" type="text" value="[% report.user.phone | html %]" name="phone" id="form_phone"> +[% END %] +[% IF NOT c.user.email_verified %] + <label for="form_username">[% loc('Email address (optional)') %]</label> + <input class="form-control" type="text" value="[% report.user.email | html %]" name="email" id="form_email"> +[% END %] <div class="form-txt-submit-box"> <input class="green-btn js-submit_register" type="submit" name="submit_register" value="[% loc('Submit') %]"> diff --git a/templates/web/base/report/new/form_user_loggedout_by_email.html b/templates/web/base/report/new/form_user_loggedout_by_email.html index 409fd4bbf..e9519f573 100644 --- a/templates/web/base/report/new/form_user_loggedout_by_email.html +++ b/templates/web/base/report/new/form_user_loggedout_by_email.html @@ -1,5 +1,9 @@ <div id="form_sign_in_no" class="form-box"> + [% IF c.config.SMS_AUTHENTICATION %] + <h5>[% loc('<strong>No</strong> Let me confirm my report by email/text') %]</h5> + [% ELSE %] <h5>[% loc('<strong>No</strong> Let me confirm my report by email') %]</h5> + [% END %] [% INCLUDE 'report/new/extra_name.html' %] [% PROCESS 'user/_anonymity.html' anonymous = report.anonymous %] @@ -22,8 +26,14 @@ <label class="inline" for="form_may_show_name">[% loc('Show my name publicly') %]</label> </div> - <label class="form-focus-hidden" for="form_phone">[% loc('Phone number (optional)') %]</label> - <input class="form-control form-focus-hidden" type="text" value="[% report.user.phone | html %]" name="phone" id="form_phone" placeholder="[% loc('Your phone number') %]"> + <div id="js-hide-if-username-phone"> + <label class="form-focus-hidden" for="form_phone">[% loc('Phone number (optional)') %]</label> + <input class="form-control form-focus-hidden" type="text" value="[% report.user.phone_display | html %]" name="phone" id="form_phone"> + </div> + <div id="js-hide-if-username-email"> + <label class="form-focus-hidden" for="form_email">[% loc('Email address (optional)') %]</label> + <input class="form-control form-focus-hidden" type="text" value="[% report.user.email | html %]" name="email" id="form_email"> + </div> <label class="form-focus-hidden" for="password_register">[% loc('Password (optional)') %]</label> diff --git a/templates/web/base/report/new/form_user_loggedout_email.html b/templates/web/base/report/new/form_user_loggedout_email.html index 39e9fd779..734eb6f35 100644 --- a/templates/web/base/report/new/form_user_loggedout_email.html +++ b/templates/web/base/report/new/form_user_loggedout_email.html @@ -1,7 +1,17 @@ -<label for="form_email">[% loc('Your email') %]</label> -[% IF field_errors.email %] - <p class='form-error'>[% field_errors.email %]</p> +[% IF c.config.SMS_AUTHENTICATION %] + [% SET username_label = loc('Your email or mobile') %] + [% SET username_type = 'text' %] + [% SET username_value = report.user.username %] +[% ELSE %] + [% SET username_label = loc('Your email') %] + [% SET username_type = 'email' %] + [% SET username_value = report.user.email %] [% END %] -<input class="form-control" type="email" value="[% report.user.email | html %]" name="email" id="form_email" placeholder="[% loc('Please enter your email address') %]" + +<label for="form_username">[% username_label %]</label> +[% IF field_errors.username %] + <p class='form-error'>[% field_errors.username %]</p> +[% END %] +<input type="[% username_type %]" value="[% username_value | html %]" name="username" id="form_username" [% IF required %]required[% END %] - class="required"> + class="form-control required"> diff --git a/templates/web/base/report/update/form_name.html b/templates/web/base/report/update/form_name.html index e4f7ac60c..f366895a5 100644 --- a/templates/web/base/report/update/form_name.html +++ b/templates/web/base/report/update/form_name.html @@ -20,8 +20,8 @@ <option value="body">[% c.user.from_body.name %]</option> [% END %] </select> - <label for="form_email">[% loc('Email address') %]</label> - <input class="form-control" name="rznvy" id="form_email" type="text" value="[% c.user.email | html %]"> + <label for="form_username">[% loc('Email address') %]</label> + <input class="form-control" name="username" id="form_username" type="text" value="[% c.user.email | html %]"> [% END %] <label for="form_name">[% loc('Name') %]</label> diff --git a/templates/web/base/report/update/form_user_loggedout_by_email.html b/templates/web/base/report/update/form_user_loggedout_by_email.html index 04a842bef..7d10fe391 100644 --- a/templates/web/base/report/update/form_user_loggedout_by_email.html +++ b/templates/web/base/report/update/form_user_loggedout_by_email.html @@ -1,5 +1,9 @@ <div id="form_sign_in_no" class="form-box"> + [% IF c.config.SMS_AUTHENTICATION %] + <h5>[% loc('<strong>No</strong> Let me confirm my update by email/text') %]</h5> + [% ELSE %] <h5>[% loc('<strong>No</strong> Let me confirm my update by email') %]</h5> + [% END %] [% INCLUDE 'report/update/form_name.html' %] diff --git a/templates/web/base/report/update/form_user_loggedout_email.html b/templates/web/base/report/update/form_user_loggedout_email.html index ccea2de02..f4228969b 100644 --- a/templates/web/base/report/update/form_user_loggedout_email.html +++ b/templates/web/base/report/update/form_user_loggedout_email.html @@ -1,7 +1,17 @@ -<label for="form_rznvy">[% loc('Your email' ) %]</label> -[% IF field_errors.email %] - <p class='form-error'>[% field_errors.email %]</p> +[% IF c.config.SMS_AUTHENTICATION %] + [% SET username_label = loc('Your email or mobile') %] + [% SET username_type = 'text' %] + [% SET username_value = update.user.username %] +[% ELSE %] + [% SET username_label = loc('Your email') %] + [% SET username_type = 'email' %] + [% SET username_value = update.user.email %] [% END %] -<input type="email" name="rznvy" id="form_rznvy" value="[% update.user.email | html %]" placeholder="[% loc('Your email address' ) %]" + +<label for="form_username">[% username_label %]</label> +[% IF field_errors.username %] + <p class='form-error'>[% field_errors.username %]</p> +[% END %] +<input type="[% username_type %]" name="username" id="form_username" value="[% username_value | html %]" [% IF required %]required[% END %] class="form-control required"> diff --git a/templates/web/bromley/report/new/form_user.html b/templates/web/bromley/report/new/form_user.html index 634e18c10..e6749f5ab 100644 --- a/templates/web/bromley/report/new/form_user.html +++ b/templates/web/bromley/report/new/form_user.html @@ -50,11 +50,11 @@ </div> [% ELSE %] - <label for="form_email">[% loc('Your email') %]</label> - [% IF field_errors.email %] - <p class='form-error'>[% field_errors.email %]</p> + <label for="form_username">[% loc('Your email') %]</label> + [% IF field_errors.username %] + <p class='form-error'>[% field_errors.username %]</p> [% END %] - <input class="form-control" type="email" value="[% report.user.email | html %]" name="email" id="form_email" placeholder="[% loc('Please enter your email address') %]" required> + <input class="form-control" type="email" value="[% report.user.email | html %]" name="username" id="form_username" placeholder="[% loc('Please enter your email address') %]" required> <div id="form_sign_in"> diff --git a/templates/web/bromley/report/update-form.html b/templates/web/bromley/report/update-form.html index e71e27528..22dfb1c08 100644 --- a/templates/web/bromley/report/update-form.html +++ b/templates/web/bromley/report/update-form.html @@ -78,14 +78,14 @@ [% ELSE %] - <label for="form_rznvy">[% loc('Email' ) %] + <label for="form_username">[% loc('Email' ) %] <span class="muted">([% loc('We never show your email') %])</span> </label> - [% IF field_errors.email %] - <p class='form-error'>[% field_errors.email %]</p> + [% IF field_errors.username %] + <p class='form-error'>[% field_errors.username %]</p> [% END %] - <input class="form-control" type="email" name="rznvy" id="form_rznvy" value="[% update.user.email | html %]" placeholder="[% loc('Your email address' ) %]" required> + <input class="form-control" type="email" name="username" id="form_username" value="[% update.user.email | html %]" placeholder="[% loc('Your email address' ) %]" required> <div id="form_sign_in"> <p>To submit your update you now need to confirm it either by email or by using a FixMyStreet password.</p> diff --git a/templates/web/fixamingata/report/new/form_user_loggedout.html b/templates/web/fixamingata/report/new/form_user_loggedout.html index 24834454c..1f7cf2aeb 100644 --- a/templates/web/fixamingata/report/new/form_user_loggedout.html +++ b/templates/web/fixamingata/report/new/form_user_loggedout.html @@ -1,8 +1,8 @@ -<label for="form_email">[% loc('Your email') %]</label> -[% IF field_errors.email %] - <p class='form-error'>[% field_errors.email %]</p> +<label for="form_username">[% loc('Your email') %]</label> +[% IF field_errors.username %] + <p class='form-error'>[% field_errors.username %]</p> [% END %] -<input type="email" class="form-control" value="[% report.user.email | html %]" name="email" id="form_email" placeholder="[% loc('Please enter your email address') %]" required> +<input type="email" class="form-control" value="[% report.user.email | html %]" name="username" id="form_username" placeholder="[% loc('Please enter your email address') %]" required> <div id="form_sign_in"> <h3>[% loc("Now to submit your report…") %]</h3> diff --git a/templates/web/zurich/report/new/fill_in_details_form.html b/templates/web/zurich/report/new/fill_in_details_form.html index 77d764950..fd21e0fff 100644 --- a/templates/web/zurich/report/new/fill_in_details_form.html +++ b/templates/web/zurich/report/new/fill_in_details_form.html @@ -50,11 +50,11 @@ [% PROCESS "report/new/category_wrapper.html" %] - <label for="form_email">[% loc('Your email') %]</label> - [% IF field_errors.email %] - <p class='form-error'>[% field_errors.email %]</p> + <label for="form_username">[% loc('Your email') %]</label> + [% IF field_errors.username %] + <p class='form-error'>[% field_errors.username %]</p> [% END %] - <input class="form-control" type="email" value="[% report.user.email | html %]" name="email" id="form_email" placeholder="[% loc('Please enter your email address') %]" required> + <input class="form-control" type="email" value="[% report.user.email | html %]" name="username" id="form_username" placeholder="[% loc('Please enter your email address') %]" required> <label for="form_name">[% loc('Name') %] [% loc('(optional)') %]</label> [% IF field_errors.name %] diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js index 25d956d61..3bf78ee31 100644 --- a/web/cobrands/fixmystreet/fixmystreet.js +++ b/web/cobrands/fixmystreet/fixmystreet.js @@ -343,8 +343,7 @@ $.extend(fixmystreet.set_up, { } ); $('#facebook_sign_in, #twitter_sign_in').click(function(e){ - $('#form_email').removeClass(); - $('#form_rznvy').removeClass(); + $('#form_username').removeClass(); $('#username').removeClass(); }); @@ -737,6 +736,19 @@ $.extend(fixmystreet.set_up, { } }, + reporting_hide_phone_email: function() { + $('#form_username').on('keyup change', function() { + var username = $(this).val(); + if (/^[^a-z]+$/i.test(username)) { + $('#js-hide-if-username-phone').hide(); + $('#js-hide-if-username-email').show(); + } else { + $('#js-hide-if-username-phone').show(); + $('#js-hide-if-username-email').hide(); + } + }); + }, + fancybox_images: function() { // Fancybox fullscreen images if (typeof $.fancybox == 'function') { diff --git a/web/cobrands/fixmystreet/staff.js b/web/cobrands/fixmystreet/staff.js index c3d1650a6..48bd36909 100644 --- a/web/cobrands/fixmystreet/staff.js +++ b/web/cobrands/fixmystreet/staff.js @@ -171,7 +171,7 @@ $.extend(fixmystreet.set_up, { var opt = this.options[this.selectedIndex], val = opt.value, txt = opt.text; - var $emailInput = $('input[name=email]').add('input[name=rznvy]'); + var $emailInput = $('input[name=username]'); var $nameInput = $('input[name=name]'); var $phoneInput = $('input[name=phone]'); var $showNameCheckbox = $('input[name=may_show_name]'); |